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

Lazy error evaluation of Result.OkIf and Result.FailIf #141

Closed
mtyski opened this issue Aug 4, 2022 · 4 comments · Fixed by #142
Closed

Lazy error evaluation of Result.OkIf and Result.FailIf #141

mtyski opened this issue Aug 4, 2022 · 4 comments · Fixed by #142

Comments

@mtyski
Copy link
Contributor

mtyski commented Aug 4, 2022

As far as I am aware, the factory methods OkIf and FailIf need the Error instances to be provided at the moment they are called.

This may pop up in scenarios, where a reported Error instance requires access to the data which might not exist:

var list = Enumerable.Range(1, 9).ToList();

// will throw an InvalidOperationException
var result = Result.FailIf(
    list.Any(IsDivisibleByTen),
    $"Item {list.First(IsDivisibleByTen)} should not be on the list!");

// snip

static bool IsDivisibleByTen(int i) => i % 10 == 0;

The proposed feature would be to implement following method overloads:

  • Result.OkIf(bool condition, Func<string>)
  • Result.OkIf(bool condition, Func<Error>)
  • Result.FailIf(bool, Func<string>)
  • Result.FailIf(bool, Func<Error>)

which would enable lazy evaluation of the errors; modifying the previous snippet:

var list = Enumerable.Range(1, 9).ToList();

// will not throw, as the Func<string> should not be called
var result = Result.FailIf(
    list.Any(IsDivisibleByTen),
    () => $"Item {list.First(IsDivisibleByTen)} should not be on the list!");

// snip

static bool IsDivisibleByTen(int i) => i % 10 == 0;

The workarounds to this are:

  • use safe methods, which do not throw exceptions in such cases or
  • use ternary / if statements.
@altmann
Copy link
Owner

altmann commented Aug 4, 2022

Looks good. Would be happy if you create a pr for this feature. I would be more happy if you also add some tests and extend the readme file with this feature. I will encrease the version number, write the changelog and generate the package.

Thanks in advance

mtyski added a commit to mtyski/FluentResults that referenced this issue Aug 4, 2022
mtyski added a commit to mtyski/FluentResults that referenced this issue Aug 4, 2022
mtyski added a commit to mtyski/FluentResults that referenced this issue Aug 4, 2022
mtyski added a commit to mtyski/FluentResults that referenced this issue Aug 4, 2022
@mtyski
Copy link
Contributor Author

mtyski commented Aug 4, 2022

Hi @altmann, thanks for a quick reply! The pull request, including overloads implementation, unit tests and a short section in the README is opened and ready for review.

@altmann
Copy link
Owner

altmann commented Aug 4, 2022

thank you for the great pr. I merged it already into the master branch. New package will be generated hopefully tomorrow.

@altmann
Copy link
Owner

altmann commented Aug 5, 2022

New package is out: https://www.nuget.org/packages/FluentResults

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 a pull request may close this issue.

2 participants