TheAngryByrd / IcedTasks

F# Cold Tasks and Cancellable Tasks
https://www.jimmybyrd.me/IcedTasks/
MIT License
120 stars 5 forks source link

Refactor Binds to Source members #12

Closed TheAngryByrd closed 1 year ago

TheAngryByrd commented 1 year ago

Proposed Changes

This uses the Source member overloads to provide a more stripped down version of the ColdTask and CancellableTask CEs.

Similar to the work I did in https://github.com/demystifyfp/FsToolkit.ErrorHandling/pull/83

Types of changes

What types of changes does your code introduce to IcedTasks? Put an x in the boxes that apply

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

TheAngryByrd commented 1 year ago

@dsyme do you know of any reason I should not use Source member overloads like this to reduce the complexity of a resumable CE?

cc @abelbraaksma @NinoFloris

abelbraaksma commented 1 year ago

Let's /cc @gusty. He mentioned to me the other day that this has been around since a long time, but only covered in the spec, not in docs. As far as I know this is safe to do, but maybe Gus can glance over these changes if he knows the intrinsics of this approach?

NinoFloris commented 1 year ago

Not as far as I know @TheAngryByrd. There are of course some overload resolution cases where having the continuation arguments in the same resolution as the value can influence the outcome to be a better fit but in almost all cases I've been able to restructure to still be able to use Source.

gusty commented 1 year ago

I don't think there's something special about Source it can be overloaded as any other methods at the risk of the scaling (logical) limitations of doing Ad-hoc overloading for the sole purpose of saving keystrokes.

abelbraaksma commented 1 year ago

@gusty I think the main use-case here is that by using Source, you'd have less boilerplate to write in the Bind and Yield functions. In other words, it's not just saving keystrokes, but making the implementation more resilient, where Source is used like a "gateway" and the rest is unified. At least that's what I take from it.

The main question at hand is: is there any caveat to using it? (also asking because of my own interest in the method).

gusty commented 1 year ago

you'd have less boilerplate to write in the Bind and Yield functions. In other words, it's not just saving keystrokes,

Well, you're saving keystrokes there already. Ideally the user should explicitly convert. Moving it to Source won't change anything in that regard.

But going to your main question:

The main question at hand is: is there any caveat to using it? (also asking because of my own interest in the method).

Not that I know. I never used it in my OSS projects, but I used it in many prototypes and I wouldn't hesitate to use it in any OSS project, it's just that I still didn't find a problem that it would solve there.

abelbraaksma commented 1 year ago

Great, thanks! Looking at the diff here, the main use case is simplification. That, of course, isn't needed in each and every CE.

TheAngryByrd commented 1 year ago

Thank you everyone for your feedback. Highly appreciated!