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

feat(Option): Support OkOrElse with materialized error #7

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

skarllot
Copy link
Contributor

@skarllot skarllot commented Jun 4, 2024

some.OkOr("Err")
    .Should()
    .BeOk(123);

[DebuggerHidden]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
[GenerateTransformer]
public static Result<T, R> OkOrElse<T, R>(
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be named OkOr to be consistent with the other names that have both a materialized and a selector fallback variant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, fixed

/// <typeparam name="T">The type of the value in the result.</typeparam>
/// <typeparam name="R">The type of the error in the result.</typeparam>
/// <param name="source">The input <see cref="Option{T}"/>.</param>
/// <param name="selector">A function that provides the default error value to return if the option is empty.</param>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the default qualifier is needed here.

Suggested change
/// <param name="selector">A function that provides the default error value to return if the option is empty.</param>
/// <param name="selector">A function that provides the error value to return if the option is empty.</param>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

/// <typeparam name="T">The type of the value in the result.</typeparam>
/// <typeparam name="R">The type of the error in the result.</typeparam>
/// <param name="source">The input <see cref="Option{T}"/>.</param>
/// <param name="invalidValue">The default error value to return if the option is empty.</param>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here:

Suggested change
/// <param name="invalidValue">The default error value to return if the option is empty.</param>
/// <param name="invalidValue">The error value to return if the option is empty.</param>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

[Test]
public void OkOr_ReturnsTheExpectedValue()
{
Option<int> some = Some(123);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's an example of where implicit casts should work 😄

Suggested change
Option<int> some = Some(123);
Option<int> some = 123;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sometimes it works 😅

Copy link
Contributor

@jeffijoe jeffijoe left a comment

Choose a reason for hiding this comment

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

Thank you!

@jeffijoe jeffijoe merged commit ab19399 into taxfyle:main Jun 4, 2024
2 checks passed
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