Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept error and bool with Must() #99

Closed
mvrahden opened this issue Apr 20, 2022 · 6 comments
Closed

Accept error and bool with Must() #99

mvrahden opened this issue Apr 20, 2022 · 6 comments

Comments

@mvrahden
Copy link

Additionally to error returns, you also often stumble upon boolean returns.

Fictional example:

func Parse(raw string) (out ConcreteType, ok bool)

It is possible to use type switches within generic function, so we could switch for error and bool alike.
Allowing the Must() function to become slightly more generic than it is right now without breaking anything.

@wirekang
Copy link
Contributor

I tried this
image
And
image

How about MustOK(Parse(""))?

@samber
Copy link
Owner

samber commented Apr 21, 2022

MustOK is good!

wirekang added a commit to wirekang/lo that referenced this issue Apr 21, 2022
@mvrahden
Copy link
Author

No need, just allow any instead of error. You throw away compile time type matching for runtime assertion. It's not a problem because people who will use Must() expect their code to panic at this point anyway.

wirekang added a commit to wirekang/lo that referenced this issue Apr 21, 2022
@wirekang
Copy link
Contributor

wirekang commented Apr 21, 2022

No need, just allow any instead of error. You throw away compile time type matching for runtime assertion. It's not a problem because people who will use Must() expect their code to panic at this point anyway.

I didn't think that far. Check out my PR. Must* panics when err is error or false. I deleted condition err != nil. Is it okay to ignore cases that err is neither error nor bool?

@mvrahden
Copy link
Author

Your PR looks good :) thank your for your support on this 💯

P.S: Just out of curiosity, why is each test case wrapped into separate blocks? I'd probably place them into individual t.Runs instead and give more context via the description. It's probably just a question of preference, but I'm curios anyway

@wirekang
Copy link
Contributor

P.S: Just out of curiosity, why is each test case wrapped into separate blocks? I'd probably place them into individual t.Runs instead and give more context via the description. It's probably just a question of preference, but I'm curios anyway

I copied and modified the existing TestMustX code. There is no other intention.

@samber samber closed this as completed Apr 22, 2022
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

No branches or pull requests

3 participants