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

ResultAsMaybe #597

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ResultAsMaybe #597

wants to merge 1 commit into from

Conversation

RorySan
Copy link

@RorySan RorySan commented Jan 17, 2025

This is a pretty simple implementation. I've explored implementations of other features of the repo and I'd need clarification regarding 3 points:

  1. Task and ValueTask implementations, should we have them? I will check similar features in depth and try to replicate them, although I'd appreciate some indications.

  2. I've seen #if NET5_0_OR_GREATER in a few places, I'm not sure if this is something I should take into account and if or how it affects this feature.

  3. Result with both Success and null value: Apparently this is possible, which I guess it makes sense if you want to imply that an operation was successful even though the result is null. As far as I understand (in a Maybe), if the Value is null, HasValue has to be false. I added a test for this but it might not make sense since it is not possible to create a Maybe containing null.

I know there's a lot missing here. Please point me in the right direction and I'll keep working on it.

@RorySan
Copy link
Author

RorySan commented Jan 17, 2025

Also, I've used TestBase instead of creating random types and data. Please confirm I'm using it properly.

@bothzoli
Copy link
Contributor

LGTM, as mentioned in #594 I think it'd make sense to add this extension to UnitResult as well.

@RorySan
Copy link
Author

RorySan commented Jan 21, 2025

Thank you @bothzoli , i'll work it out and update this. What about the other points i mention in the PR?

@bothzoli
Copy link
Contributor

Oh yeah, sorry:

1 - I'll pass on this one.
2 - I think you'll only need these if you do the Task implementations.
3 - Yeah, this is a bit tricky. On the one hand this makes sense from the Maybe-s perspective, yet it feels a bit weird that a Success turns into a None...
+1 - The usage of TestBase looks good to me here.

(I'm also merely another enthusiastic contributor to this repo, so I'm sorry I can't give more exact answers :))

@RorySan
Copy link
Author

RorySan commented Jan 21, 2025

Thanks for your input, may be @vkhorikov can help us out with task / value task. In the meantime i'll see about UnitResult and update the PR.

@RorySan
Copy link
Author

RorySan commented Jan 21, 2025

@bothzoli I've looked into UnitResult but by definition it Represents the result of an operation that has no return value on success, or an error on failure. I don't see how we can turn that into a Maybe when there is no value for the Some case. Even if we were to return Unit inside that Maybe it kinda defeats the purpose of a Maybe. Can you clarify this?

@bothzoli
Copy link
Contributor

Ahh yeah, now that I think about it, you're perfectly right. By definition a UnitResult cannot have a value, so it cannot become a Some value 🤦
Sorry for making you run this circle in vain... 😔

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.

2 participants