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

Async map & bind #44

Open
nojaf opened this issue Nov 17, 2017 · 23 comments
Open

Async map & bind #44

nojaf opened this issue Nov 17, 2017 · 23 comments
Milestone

Comments

@nojaf
Copy link

nojaf commented Nov 17, 2017

I'm having some code that needs to transform an Option using async functions.
But it is getting out of hand.

public async Task<Option<GetMaterialFlowResponse, string>> Handle(int id)
{
    var externalFlowId = (await 
            _context.AuthenticFlows
                .Where(a => a.Id == id)
                .Select(a => a.Id).ToArrayAsync())
                .SingleOrNone();

    var staticMeta = await _flowWebService.GetMetaDataAsync();
    var materialMeta = staticMeta.FirstOrNone(m => m.LogicType.ToLower() == "material" && m.ConditionsFields.Any(c => c.Name ==  "MsgTo"));

    foreach (var meta in materialMeta)
    {
        foreach (var flowId in externalFlowId)
        {
            var flowDetail = await GetFlowDetail(flowId);
            foreach (var detail in flowDetail)
            {
                var msgToId = meta.ConditionsFields.First(c => c.Name == "MsgTo").ID;
                var msgToOption = detail.Conditions.FirstOrNone(c => c.ConditionFieldID == msgToId).Map(c => c.Value);
                foreach (var msgTo in msgToOption)
                {
                    var outputsOption = await new GetFlowOutputFormatsHandler(_messageTypeMetaDataService, _rampService).Handle(msgTo);
                    foreach (var outputs in outputsOption)
                    {
                        var conditions = detail.Conditions.Select(c =>
                            new FlowConditionItem((int) c.ConditionFieldID, (int) c.ConditionOperatorID,
                                c.Value)).ToArray();
                        return Option.Some<GetMaterialFlowResponse, string>(
                            new GetMaterialFlowResponse(detail.Name, conditions, outputs));
                    }
                    return Option.None<GetMaterialFlowResponse, string>(
                        $"Could not determine output types for {msgTo}");
                }
                return Option.None<GetMaterialFlowResponse, string>("No MsgTo found in flow");
            }
            return Option.None<GetMaterialFlowResponse, string>($"No flow found in the WCF service with Id {flowId}");
        }
        return Option.None<GetMaterialFlowResponse, string>($"No flow found for authentic Id {id}");
    }

    return Option.None<GetMaterialFlowResponse, string>("No meta data found for material");
}

In the most inner foreach I'm using an early return to return the happy path but this is getting out of hand and just messy.
Is there any way to improve the code?

@nlkl
Copy link
Owner

nlkl commented Dec 17, 2017

Hi,

Unfortunately, using Optional with async/Task is currently rather clumsy. Improving this (via Optional.Async) is on the very top of my TODO/roadmap for the library.

For now, the main way to reduce nesting when working with async, is to switch to a more imperative style. Unfortunately, this approach provides fewer safety guarantees. An alternative would be to split the method into multiple methods (if you are using C# 7+ local methods would be an option here).

Example (I haven't compiled or executed the code, so it might need a bit of tweaking for it to actually run):

public async Task<Option<GetMaterialFlowResponse, string>> Handle(int id)
{
    var externalFlowId = (await 
            _context.AuthenticFlows
                .Where(a => a.Id == id)
                .Select(a => a.Id).ToArrayAsync())
                .SingleOrNone();

    if (!externalFlowId.HasValue) return Option.None<GetMaterialFlowResponse, string>($"No flow found for authentic Id {id}");
    var flowId = externalFlowId.ValueOrFailure();

    var staticMeta = await _flowWebService.GetMetaDataAsync();
    var materialMeta = staticMeta.FirstOrNone(m => m.LogicType.ToLower() == "material" && m.ConditionsFields.Any(c => c.Name ==  "MsgTo"));
    if (!materialMeta.HasValue) return Option.None<GetMaterialFlowResponse, string>("No meta data found for material");
    var meta = materialMeta.ValueOrFailure();

    var flowDetail = await GetFlowDetail(flowId);
    if (!flowDetail.HasValue) return Option.None<GetMaterialFlowResponse, string>($"No flow found in the WCF service with Id {flowId}");
    var detail = flowDetail.ValueOrFailure();

    var msgToId = meta.ConditionsFields.First(c => c.Name == "MsgTo").ID;
    var msgToOption = detail.Conditions.FirstOrNone(c => c.ConditionFieldID == msgToId).Map(c => c.Value);
    if (!msgToOption.HasValue) return Option.None<GetMaterialFlowResponse, string>("No MsgTo found in flow");
    var msgTo = msgToOption.ValueOrFailure();

    var outputsOption = await new GetFlowOutputFormatsHandler(_messageTypeMetaDataService, _rampService).Handle(msgTo);
    if (!outputsOption.HasValue) return Option.None<GetMaterialFlowResponse, string>($"Could not determine output types for {msgTo}");
    var outputs = outputsOption.ValueOrFailure();

    var conditions = detail
        .Conditions
        .Select(c => new FlowConditionItem((int) c.ConditionFieldID, (int) c.ConditionOperatorID, c.Value))
        .ToArray();
    return Option.Some<GetMaterialFlowResponse, string>(new GetMaterialFlowResponse(detail.Name, conditions, outputs));
}

As mentioned in the beginning, Optional.Async should improve the situation a lot, and is currently the highest prioritized upcoming feature. If you feel adventurous, you might want to look into the current Optional.Async implementation, and borrow a few snippets from there, until the functionality is officially released.

Hope it helps - and feel very free to ask if further clarification is needed.

Have a nice day.

Best regards,
/ Nils

@nojaf
Copy link
Author

nojaf commented Jan 4, 2018

I ended up making some extension methods of my own.

        public static Option<TOutput> Bind<TInput, TOutput>(this Option<TInput> option, Func<TInput, Option<TOutput>> fn)
        {
            return option.Match(fn, Option.None<TOutput>);
        }

        public static Option<TOutput, TError> Bind<TInput, TError, TOutput>(this Option<TInput, TError> option, Func<TInput, Option<TOutput, TError>> fn)
        {
            return option.Match(fn, Option.None<TOutput, TError>);
        }
        
        public static async Task<Option<C, B>> Bind<A, B, C>(this Task<Option<A, B>> option,
            Func<A, Option<C, B>> fn)
        {
            return (await option).Bind(fn);
        }
        
        public static async Task<Option<C, B>> BindAsync<A, B, C>(this Option<A, B> option,
            Func<A, Task<Option<C, B>>> fn)
        {
            return await option.Match(async input => await fn(input), c => Task.FromResult(Option.None<C, B>(c)));
        }

        public static async Task<Option<C, B>> BindAsync<A, B, C>(this Task<Option<A, B>> option,
            Func<A, Task<Option<C, B>>> fn)
        {
            return await (await option).BindAsync(fn);
        }
        
        public static async Task<Option<C, B>> MapAsync<A, B, C>(this Option<A, B> option, Func<A, Task<C>> fn)
        {
            var mapped = option.Map(fn);
            return await mapped.Match(
                async c => Option.Some<C, B>(await c),
                b => Task.FromResult(Option.None<C, B>(b))
            );
        }
        
        public static async Task<Option<C, B>> MapAsync<A, B, C>(this Task<Option<A, B>> option, Func<A, Task<C>> fn)
        {
            var mapped = (await option).Map(fn);
            return await mapped.Match(
                async c => Option.Some<C, B>(await c),
                b => Task.FromResult(Option.None<C, B>(b))
            );
        }

        public static async Task<Option<C, B>> Map<A, B, C>(this Task<Option<A, B>> option, Func<A, C> fn)
        {
            var mapped = (await option).Map(fn);
            return mapped;
        }

Any news on Optional.Async?

@rydergillen-compacSort
Copy link

rydergillen-compacSort commented Feb 9, 2018

I wanted to chime in on this as I too have had trouble with Option/Async & code bloat. After much trial and error I came up with an alternative class for handling Tasks w/Optional flows.

	public class TaskOption<T>
	{
		private readonly Func<Task<T>> _task;

		public TaskOption(Task<T> task)
			: this(() => task)
		{ }

		public TaskOption(Func<Task<T>> task)
		{
			this._task = task ?? throw new ArgumentNullException(nameof(task));
		}

		public TaskOption<T> Filter(Predicate<T> filterPredicate, Func<T, Exception> exceptionalFunc)
		{
			var filtered = this.Match(
				some: s => filterPredicate(s) ? s : throw exceptionalFunc(s),
				none: n => throw n);

			return TaskOption.Create(filtered);
		}

		public TaskOption<TResult> Map<TResult>(Func<T, TResult> mapping) =>
			this._task().ContinueWith(t => mapping(t.Result));

		public TaskOption<TResult> Map<TResult>(Func<T, Task<TResult>> mapping) =>
			this._task().ContinueWith(t => mapping(t.Result)).Unwrap();

		public Task<TResult> Match<TResult>(Func<T, TResult> some, Func<Exception, TResult> none) => this._task()
			.ContinueWith(t =>
			{
				if (t.IsCanceled)
				{
					return none(new TaskCanceledException(t));
				}

				if (t.IsFaulted)
				{
					return none(t.Exception);
				}

				return some(t.Result);
			});

		#region Await

		public TaskAwaiter<Option<T, Exception>> GetAwaiter()
		{
			var continued = this._task().ContinueWith(t =>
			{
				if (t.IsCanceled)
				{
					return Option.None<T, Exception>(new TaskCanceledException(t));
				}

				if (t.IsFaulted)
				{
					return Option.None<T, Exception>(t.Exception);
				}

				return Option.Some<T, Exception>(t.Result);
			});

			return continued.GetAwaiter();
		}

		public ConfiguredTaskAwaitable<Option<T, Exception>> ConfigureAwait(bool continueOnCapturedContext)
		{
			var continued = this._task().ContinueWith(t => {
				if (t.IsCanceled)
				{
					return Option.None<T, Exception>(new TaskCanceledException(t));
				}

				if (t.IsFaulted)
				{
					return Option.None<T, Exception>(t.Exception);
				}

				return Option.Some<T, Exception>(t.Result);
			});

			return continued.ConfigureAwait(continueOnCapturedContext);
		}

		#endregion

		#region Operators

		public static implicit operator Task<T>(TaskOption<T> option) => option._task();

		public static implicit operator TaskOption<T>(Task<T> task) => new TaskOption<T>(task);

		#endregion
	}

@rydergillen-compacSort
Copy link

rydergillen-compacSort commented Feb 9, 2018

This....

var externalFlowId = (await 
            _context.AuthenticFlows
                .Where(a => a.Id == id)
                .Select(a => a.Id).ToArrayAsync())
                .SingleOrNone();
if (!externalFlowId.HasValue) return Option.None<GetMaterialFlowResponse, string>($"No flow found for authentic Id {id}");
    var flowId = externalFlowId.ValueOrFailure();

Is refactored to this...

   var externalFlowId = _context.AuthenticFlows
                .Where(a => a.Id == id)
                .Select(a => a.Id)
                .ToArrayAsync()
                .ToTaskOption()
                .SingleOrNone(() => new Exception($"No flow found for authentic Id {id}"))
                .Map(flowId => _flowWebService.GetMetaDataAsync().ToTaskOption()... rest of method goes here)

@nojaf
Copy link
Author

nojaf commented Feb 9, 2018

@rydergillen-compacSort interesting! Do you also have a version with Option<A, B>?
I'm using a lot of railway oriented programming in my code.

@rydergillen-compacSort
Copy link

rydergillen-compacSort commented Feb 9, 2018

TaskOption<T> is a hybrid, the 2nd type argument B is implied as Exception. This is necessary to handle Task<T>.Exception vs. Task<T>.Result otherwise the Exception would be suppressed and/or each TaskOption<T> would need a transformation method for mapping Exception ==> B type, requiring an additional argument for each method call (yuck).

Option<A, B> is obtained via the await keyword, then B type can be mapped to something other than Exception without implicit data loss.

await to return to normal Option<T, Exception> then MapException()
Task<T> task = taskOption; //also can be directly assigned to Task<>
Option<T, Exception> option = await taskOption;
Option<T, string> stringErrorOption = option.MapException(ex => ex.Message);
Option<T> emptyErrorOption = option.WithoutException();

@nlkl
Copy link
Owner

nlkl commented Mar 3, 2018

Hi,

First things first - it is great to see some discussions on this topic!

Secondly, I just wanted to give an update on the current state of Optional.Async, as it is slowly starting to take shape.

First, let me share a few design decisions:

  • I wasn't satisfied with the original concept of introducing an AsyncOption class - although this approach had some advantages, I find it more complex and cumbersome than necessary. Instead, I am planning on simply letting Optional.Async be a set of extension methods on Task<Option<T>> (and a few on Option<T> for interop).
  • The above mentioned extension methods will cover all "transform operations" - that is operations that transform an option into another option. This includes Map, FlatMap, Filter, Or, Else, and a few others.
  • However, it does not include operations to extract values out of options, such as Match, ValueOr, Contains, etc. Whenever an option needs to be "opened up", simply await the task instead to get the actual option - which is really the "right" thing to do anyway. Optional.Async is about improving composition, which is the real pain point at the moment - re-exposing all operations provide little value in practice, but adds quite a bit of complexity and maintenance overhead.
  • Where relevant, operations can be configured to either execute on the captured synchronization context or not. By default, the synchronization context is not captured, as this is generally considered the better/safer default.
  • Option<T> and Option<T, TException> will both be equally supported.
  • There will be full support for linq query syntax on Task<Option<T>>, e.g.:
return
    from report in TryLoadReportAsync(reportId)
    where report.Date >= reportPeriodStartDate
    from renderedReport in TryRenderReportAsync(report)
    select renderedReport;

However, this comes with a few caveats. First of all, it doesn't offer the flexibility of the full set operations - e.g. optionTask.Filter(...) can take an async predicate, which is not allowed in the Linq query syntax. Similarly, the Linq query syntax doesn't allow enabling/disabling sync context capture. I am yet to make a final decision on this (and would love your input), but currently I'm planning on only providing Linq query syntax for the default case (not capturing the context) - the alternative would be to provide both, in different namespaces, but I am not sure I like this idea.

Current state

The bulk of the functionality has been implemented, with the exception of what is mentioned below.

However, the functionality still needs thorough test coverage - particularly so, due to the many different variations of the new async extension methods, and the complexity related to the sync context capture.

Finally, XML documentation also need to be added before an actual release.

As such, the actual release is still some time away. I might, however, throw out a pre-release some time before everything is completely polished.

What is yet to be decided

  • Whether or not to provide Linq extensions for the default case only (not capturing sync context) or for both cases (in different namespaces)
  • Whether or not to provide convenience wrapper methods (and which), e.g. something like:
Task<Option<T>> ToAsync(this Option<T> option);
Task<Option<T>> SomeAsync<T>(this Task<T>);
Task<Option<T>> SomeNotNullAsync<T>(this Task<T>);
Task<Option<T>> SomeWhenAsync<T>(this Task<T>, <predicate/async predicate>);
Task<Option<T>> NoneAsync<T>(this Task<T>);
Task<Option<T>> NoneWhenAsync<T>(this Task<T>, <predicate/async predicate>);

Final remarks

Comments, thoughts, suggestions, etc., are all very welcome. Also, feel free to check out the implementation on the development branch (https://github.com/nlkl/Optional/tree/develop).

@JudahGabriel
Copy link

JudahGabriel commented Mar 13, 2018

Where relevant, operations can be configured to either execute on the captured synchronization context or not. By default, the synchronization context is not captured, as this is generally considered the better/safer default.

Thank you! Otherwise, this would render async optionals useless for ASP.NET [Core], where you must resume on the sync context.

Whether or not to provide Linq extensions for the default case only (not capturing sync context) or for both cases (in different namespaces)

Without that, we couldn't use Linq extensions for Optional in our controllers. That would really suck. Please add either a different namespace, or even a global initialization flag, e.g. Optional.CaptureOnSyncContext.

Whether or not to provide convenience wrapper methods (and which), e.g. something like:

That'd be helpful. I think the biggest pain point with Optional is the syntactical overhead. Anything that we can do to reduce that would be beneficial.

@nlkl
Copy link
Owner

nlkl commented Mar 14, 2018

Hi @JudahGabriel

Great to get some feedback on this. A few thoughts:

I agree that it is important to support capturing the context. I think the alternate namespace is probably the most plausible option for the Linq extensions - I am not too much of a fan of a global flag, as it is a bit too implicit to my taste (and likely introduces the need for synchronization to keep it thread safe, or at least some strict guidelines as to when to set it). Separate namespaces also make it possible to choose the right implementation in each situation - although this is a rather double edged sword, as it definitely makes it less obvious what is going on (one would need to actually check the namespace).

ASP.NET Core doesn't actually have a synchronization context, so .ConfigureAwait(false) is basically a no-op in this case, and capturing/ignoring the context should make no difference. I have been using .ConfigureAwait(false) on ASP.NET Core projects for some time now, and had no problems (and similarly for the opposite situation). For classical ASP.NET, Winforms, WPF, and so forth, it is of course still relevant.

Any thoughts on this being made into a set of extension methods, instead of an actual AsyncOption<T>?

/ Nils

@JudahGabriel
Copy link

Sync context: hmmm, that's strange. I had some ASP.NET Core code that talked to a database asynchronously. Using optional + old Optional.Async code didn't work...until I .ConfigureAwait(false). (Or maybe I'm remembering classical ASP.NET? Hrmmm.)

I'll have to retest that. If indeed sync contexts aren't needed, this becomes less of an issue for me; virtually all my web projects are now AspNetCore.

Extension methods: yeah, I like that. I think it would reduce some redundancy on your part. Maybe it would ease composition as well? I'd really have to try it out to say.

@JudahGabriel
Copy link

JudahGabriel commented Mar 14, 2018

p.s. One other feedback item:

I have been bitten by this on more than one occasion:

// Prints true!
Option.Some("hello").Map(h => default(string)).HasValue

Basically, propagating a null inside an Optional -- a weird possibility that, IMO, undermines the primary purpose of the library -- can and does lead to runtime surprises. I often find myself littering my optional clauses with .NotNull() out of paranoia. I know it's been discussed before, but I thought I'd reiterate this pain point now that we're on 4.0.

@jblackburn21
Copy link

@nlkl Would it be possible to a get NuGet package with the current async extensions from the develop branch?

@pizycki
Copy link

pizycki commented Oct 16, 2018

Any update on releasing async extensions in nuget package?

@nlkl
Copy link
Owner

nlkl commented Dec 12, 2018

Just wanted to let you know, that I packaged a pre-release version of the Optional.Async package, and am ready to upload it.

However, in the meanwhile, the Optional.Async package name has been taken, which prevents me from doing it right now. I have contacted the package owner, and hopefully we will figure something out.

I will keep you posted once I know more.

@bqstony
Copy link

bqstony commented Dec 20, 2018

Any news here?

@pizycki
Copy link

pizycki commented Feb 5, 2019

Any update? It's been almost 3 months already.

@nlkl nlkl added this to the 5.0.0 milestone Feb 5, 2019
@nlkl
Copy link
Owner

nlkl commented Feb 5, 2019

Hi everyone,

Partially due to the above, and partially because async is as widely used as it is, I have decided not to release a separate Optional.Async package.

Instead, the async extensions will be part of Optional itself going forwards (although excluded from the .NET 3.5 version), starting from version 5.0.0.

As such, I have decided to release an alpha version of Optional 5.0.0, which contains the following new features:

Let me know what you think.

Best regards,
Nils

@dnikolovv
Copy link

Perfect!

I've updated the Optional.Async package, noting that it's deprecated.

@dnikolovv
Copy link

I just tried migrating some of our services to using Optional 5.0, but ran into the following missing extensions:

SomeWhenAsync<T>
SomeWhenAsync<T, TException>

SomeNotNullAsync<T>
SomeNotNullAsync<T, TException>

We also use FilterAsync<T, TException>, which is basically WithException + Filter. What are your thoughts on adding that?

@bqstony
Copy link

bqstony commented Mar 18, 2019

I Would like to see a release 🤩

@RaringCoder
Copy link

Any word on when v5 is going to be released?

@bbqchickenrobot
Copy link

bbqchickenrobot commented Feb 26, 2023

I know it's been some years but out of curiosity: Any updates on v5? Is it fine to use dnikolvv's version until then (signature compatibility, etc)?

@LukasKathrein
Copy link

Would also be interested in the status of a potential v5 release. Can you share some insights @nlkl?

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