Skip to content

chore: change the signature of the Convert and MustConvert#78

Merged
ccoVeille merged 1 commit intomainfrom
convert-signature
Feb 15, 2025
Merged

chore: change the signature of the Convert and MustConvert#78
ccoVeille merged 1 commit intomainfrom
convert-signature

Conversation

@ccoVeille
Copy link
Owner

These functions were accepting any as argument, which was not really helpful.

Some types were not supported, and the users had to figure out by himself at the runtime.
The unsupported types are now reported at the compilation time.

This is a breaking change, but it's the whole point of this commit.

BREAKING CHANGE: a minor one but still breaking one.

Fixes #76

These functions were accepting `any` as argument, which was not really helpful.

Some types were not supported and the users had to figure out by himself at the runtime.
The unsupported types are now reported at the compilation time.

This is a breaking change, but it's the whole point of this commit.

BREAKING CHANGE: a minor one but still breaking one.
@ccoVeille
Copy link
Owner Author

Thank you @c4rlo for inspiring me this change

@codecov
Copy link

codecov bot commented Feb 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.78%. Comparing base (36ee395) to head (5ec6d96).
Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##              main      #78      +/-   ##
===========================================
- Coverage   100.00%   98.78%   -1.22%     
===========================================
  Files            3        3              
  Lines          247      247              
===========================================
- Hits           247      244       -3     
- Misses           0        3       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ccoVeille
Copy link
Owner Author

ccoVeille commented Feb 1, 2025

The regression in the coverage is expected

https://app.codecov.io/gh/ccoVeille/go-safecast/pull/78/blob/conversion.go

image

It's now impossible to pass something that is not supported, and that's the whole idea of the refactoring.

Because of the type check switch, and the fact each type needs to be handled in its own case, I didn't find a way (yet) to provide code that could avoid this regression

func Convert[NumOut Number](orig any) (converted NumOut, err error) {
switch v := orig.(type) {
func Convert[NumOut Number, NumIn Input](orig NumIn) (converted NumOut, err error) {
switch v := any(orig).(type) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if orig is of a type A declared as type A int? In that case, A satisfies Input so it compiles, but I believe this switch statement won't find a match and hence this function returns an error.

Copy link
Owner Author

@ccoVeille ccoVeille Feb 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's the first step of a refactoring.

Here I'm only applying a part of the refactoring that need to be done.

I'm only addressing the "the function accepts any" issue that you raised a few weeks ago.

I still have to deal with "type alias are not working" issue. See #72

I have code ready locally.

But I wanted this PR first.

@ccoVeille ccoVeille merged commit 0b087ea into main Feb 15, 2025
4 of 5 checks passed
@ccoVeille ccoVeille deleted the convert-signature branch February 15, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Breaking Change: Consider updating Convert and MustConvert signature

2 participants