rspeele / TaskBuilder.fs

F# computation expression builder for System.Threading.Tasks
Creative Commons Zero v1.0 Universal
235 stars 27 forks source link

Async overload bind #11

Closed dustinmoris closed 6 years ago

dustinmoris commented 6 years ago

Hi,

Do you think we could add an Async overload bind to the TaskBuilder?

It would be mainly for convenience to save an Async.StartAsTask as seen here.

Related to: https://github.com/giraffe-fsharp/Giraffe/issues/225

rspeele commented 6 years ago

I will go ahead and add this this weekend as long as it doesn't cause any type inference problems for existing code (I can't imagine why it would, but I will check).

cmeeren commented 6 years ago

I don't think it will unless there are ambiguous types (I have run into them in the case of a CE that worked with both Async<_> and Async<Result<_,_>>), but if there are, you can work around it by adding the overload as an extension on the TaskBuilder class instead of as a proper member:

// Normal members
type TaskBuilder() =
  member ...

// Ambiguous overloads that should not take precedence
type TaskBuilder with
  member ...
dustinmoris commented 6 years ago

Nice, thank you!

rspeele commented 6 years ago

A Nu NuGet will soon be available to be NuGotten.

https://www.nuget.org/packages/TaskBuilder.fs/1.1.0

gusty commented 6 years ago

What about introducing conversion functions instead of adding "magic overloads". In contrast to C# where everything is automatically converted, in F# is not usual to have magic conversion happening all the time. It's better to be explicit, makes easy to reason about the types and the code.

cmeeren commented 6 years ago

I generally agree that explicit is good, but within reason:

  1. If we were to be entirely explicit about everything we'd have to use assembly. Abstractions and other conveniences are, in general, necessary in order to be productive.
  2. Magic is a matter of definition and knowledge ("sufficiently advanced technology is indistinguishable from magic" and all that).
  3. The overloads that were added are completely normal overloads, not extension members like I mentioned in my previous comment.
  4. Even such "extension overloads" follow strict and well defined rules as per the F# spec, does not cause any ambiguity, does not break type safety, and reduce boilerplate (and IMHO are not confusing, see point 2).

We already had a conversion function, Async.StartAsTask, but that was what we tried to get away from.

gusty commented 6 years ago

Thanks @cmeeren see my comments below

If we were to be entirely explicit about everything we'd have to use assembly. Abstractions and other conveniences are, in general, necessary in order to be productive.

I like abstractions too, but good ones. Ad-hoc overloading (when it's really ad-hoc) doesn't abstract anything, on the contrary.

Magic is a matter of definition and knowledge ("sufficiently advanced technology is indistinguishable from magic" and all that).

When I say magic, I refer to something that works in an arbitrary scenario but not in all others, this happens without following any specific rule unless I see the implementations, where I will find what you describe in your points 3 and 4, but then by doing so the abstraction (your point 1) is over.

rspeele commented 6 years ago

I agree that it's not a clean design to special-case this, and it could be confusing. At the same time, this project is fundamentally about convenience -- it is essentially a replacement for using async {} with Async.AwaitTask sprinkled throughout. As a bonus, performance is a little better too, but that's not the main reason for TaskBuilder to exist. After all, if performance is critical then C# async methods are still best.

Since convenience is the goal, I think having these overloads included "out of the box" makes sense.