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

Add OnSuccess and OnFailure fluent extensions #12

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

Conversation

GoodeUser
Copy link

@GoodeUser GoodeUser commented May 17, 2020

These extension methods let you chain Outcomes together in a fluent manner. For example:

using Ether.Outcomes; // This is required for the `OnSuccess` and `OnFailure` extension methods

public class ApiValidator
{
    public IOutcome<ApiUser> Validate(HttpContext httpContext)
    {
        // You only need to use `await` here if one of your methods chained below are Async/return a Task
        return await GetAuthenticationHeader(httpContext)
            .OnSuccess(GetUserIdFromToken)
            .OnSuccess(GetUser)
            .OnSuccess((user) => VerifyUserIsActive(user, httpContext));
    }

    public IOutcome<string> GetAuthenticationHeader(HttpContext httpContext)
    {
        if (SomeCondition)
            return Outcomes.Failure();

        return Outcomes.Success("HeaderValue");
    }

    public IOutcome<Guid> GetUserIdFromToken(string token)
    {
        if (SomeCondition)
            return Outcomes.Failure();

        return Outcomes.Success(Guid.NewGuid());
    }

    public async Task<IOutcome<ApiUser>> GetUser(Guid userId)
    {
        if (SomeCondition)
            return Outcomes.Failure();

        return Outcomes.Success(await Repository.GetUser(userId));
    }

    public IOutcome<ApiUser> VerifyUserIsActive(Guid userId, HttpContext pretend)
    {
        if (SomeCondition)
            return Outcomes.Failure();

        return Outcomes.Success(new ApiUser());
    }
}

The fact that they are extension methods and not a part of the IOutcome interface lets you work with Task<IOutcome> much like you would IOutcome which really simplifies the interface for doing async validations. I'm very curious on your thoughts for these - do you think they belong here/would benefit the users of this package?

P.S. Thanks for this library. I use this library in practically all of my projects (along with my extension methods of course). It's a great way to encapsulate the logic for some computation to fail.

@kinetiq
Copy link
Owner

kinetiq commented May 18, 2020

Hey @GoodeUser, thanks for the contribution and the kind words! Async support is indeed something that is not well handled by Outcomes currently and I would love to fill that hole.

It touches on another issue... I've been playing with a bunch of approaches for cleaner handling of some common patterns. I sometimes end up with a lot of code like:

var step1Outcome= doStep1();

if (step1Outcome.Failure())
 return Outcome.Failure()
               .WithMessage("There was a problem with Step1...)
               .WithMessagesFrom(step1Outcome);

var step2Outcome = doStep2();

if (step2Outcome.Failure()) 
 return Outcome.Failure()
               .WithMessage("There was a problem with Step2...)
               .WithMessagesFrom(step2Outcome);

var step3Outcome = doStep3();

if (step3Outcome.Failure()) 
 return Outcome.Failure()
               .WithMessage("There was a problem with Step3...)
               .WithMessagesFrom(step3Outcome);

var someResult = step3Outcome.Value;

return Outcomes.Success(someResult);

It's a net win because you can roll up messages in useful ways, but the code feels a little verbose. I would love something like, perhaps as an optional additional package:

return doStep1()
 .Then(doStep2())
 .Then(doStep3())
 .OnAnyFailure([some API for handling failures])
 .OnAllSuccess([some API for building up the final outcome, maybe assigning a value, etc])

Curious if you also run into that situation, and if so, how you feel about it? I'm currently thinking the semantics you're using are on the right track, but if I do something like that, I want to try and address these other issues at the same time.

I will check your code out in more detail early this week. So far I've only looked at your actual message, and I skimmed enough to see that you added some unit tests, etc, which is great.

@GoodeUser
Copy link
Author

GoodeUser commented May 19, 2020

Yes, the kind of code where I find myself writing those chained if (outcome.Failure()) statements is exactly what I was trying to solve with these extension methods. I find myself often wanting to either chain operations either on the success of the operations or on the failure of the operations.

It's pretty much always one-sided though. I either want to make sure all operations are successful (probably the most common scenario) or I want to continue through failure cases until I get to a successful one. These extension methods let me do precisely that.

For the scenario where you want to continue on ensuring all the operations are successful and want to stop at the first failure(like some validation on a field, verifying that a user is active, etc.):

return doStep1()
    .OnSuccess((step1Value) => doStep2(step1Value))
    .OnSuccess((step2Value) => doStep3(step2Value));

OR more concisely:

return doStep1()
    .OnSuccess(doStep2)
    .OnSuccess(doStep3);

There is also the scenario where you want to find an operation that is successful (such as API validation where you have different methods and want to get to the first working one to prove that a user can access a resource).

return TryBasicTokenAuth(httpContext)
    .OnFailure(() => TrySecretKeyAuth(httpContext))
    .OnFailure(() => TryPostBodyAuth(httpContext))
    .OnFailure(() => TrySecretQueryStringParam(httpContext));

I also program in F# and when I'm in F# land I can use the Either or Result data type for these kinds of computations which effectively models a success/failure case (I see Outcome.NET library as a simpler version of that data type minus the ability to chain operations and more Object Oriented). The closest library I've found to this one is Operation which seems more focused on Functional Programming in C# rather than being easy to use. Anyways, in F# you would Map the left (which I renamed OnSuccess) or Map the right (which I renamed OnFailure). My goal is to bring that same functionality to this library and make it fit better in OOP land.

Feel free to check out the code and let me know what you think. I love feedback and would be more than happy to make any changes you'd like done here.

@kinetiq
Copy link
Owner

kinetiq commented May 20, 2020

Was buried today, but I wanted to check in and say that I really like your thoughts on this.

I want to make sure the common cases I'm aware of are handled and see what they will look like... Maybe look at a few slight variations on word choice and see how they feel, to be diligent. There's also a situation where if anything fails, I want to get access to the failure outcome and maybe have a chance to format it, but I also don't want to create an overly complicated API, so that might be best handled outside of this... Requires a little thought with a clearer mind than I have right now, at 2am. :)

There's another semi-related pattern that comes up sometimes, especially in validation scenarios, where you want to roll up a bunch of messages, like this:

var errors = new List<string>();

var step1Outcome = DoStep1;

if(step1Outcome.Failure)
 errors.AddRange(step1Outcome.Messages);

var step2Outcome = DoStep2();

if(step2Outcome.Failure)
 errors.AddRange(step2Outcome.Messages);

if (errors.Any())
 return Outcomes.Failure()
                .WithMessagesFrom(errors);

return Outcomes.Success();

Curious if you ever run into this one?

@GoodeUser
Copy link
Author

I have run into that one a few times actually but never noticed the pattern there. Interesting. Yeah, I've used this pattern to validate user input before running it through some sort of business logic. The code sample you wrote with doStep1().Then(doStep2()) or some method which would seem to run on either case would solve that.

I'm super interested to see what you come up with and am more than happy to help so let me know if there is anything you'd like me to help implement.

@kinetiq
Copy link
Owner

kinetiq commented May 31, 2020

Still haven't forgotten about this. I had a huge release at Roster and then I needed to recover a little. I'm going to try and dive into this next week.

@kinetiq
Copy link
Owner

kinetiq commented Apr 14, 2023

@GoodeUser It has been a long time, and all I can say is that Covid was rough. Do you still have any interest in working on this?

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