pharo-contributions / taskit

TaskIt is a library that ease Process usage in Pharo. It provides abstractions to execute and synchronize concurrent tasks, and several pre-built mechanisms that are useful for many application developers.
MIT License
43 stars 24 forks source link

new futur combinator suggestion #firstSuccessfulCompleteOf:. #91

Closed mattonem closed 3 years ago

mattonem commented 4 years ago

Hello guys, Could you please consider this contribution ? or just discuss about my usecase...

My usecase:

I think this #firstSucccessfulCompleteOf: is a cool addiction to taskIt futur combinators and could come handy in other usecases.

Although the error handling I implemented is terrible...

Please let me know what you think about it.

regards max

tinchodias commented 3 years ago

Oh, too bad we didn't review this before... Sorry @mattonem, I didn't analyze the codes but I think @noha already covered this functionality with TKTFuture>>any: in https://github.com/pharo-contributions/taskit/pull/80. I'd be great if you can check if I'm right. Maybe there is something to complement.

tinchodias commented 3 years ago

Here: https://github.com/pharo-contributions/taskit/pull/80/files#diff-8a7acf4d9ac8027daae12831367f8ddad36a5518c4d00c5176bf218b8f0ee32bR180

tinchodias commented 3 years ago

I looked again with more detail and I think it can be valuable to have both. Norbert's any: is class side and receives a collection of futures, while @mattonem's firstSucccessfulCompleteOf: is instance-side, of self "against" another received as argument.

mattonem commented 3 years ago

Thanks @tinchodias. It was a while ago so I don't remember the exact details of the contribution... But I think the error handling was pretty bad... I'll take a look at @noha contribution and align on how ever he implemented it.

mattonem commented 3 years ago

Btw behaviour of any: is not exactly the same as it would return the first future to finish -- success or failure. The purpose of firstSuccesfulCompleteOf: is also to ignore failure and keep the futures running until a success pop's up.

mattonem commented 3 years ago

Now we could also consider implementing a class side method anySucessful:. Would that be useful?

noha commented 3 years ago

Looking at it again the implementation of any is wrong. Any is supposed to succeed on first success of any future and fail only if all failed. I can correct this

noha commented 3 years ago

I corrected the any future. Have a look at #104

noha commented 3 years ago

Btw. what is the difference between this and firstCompleteOf: ?

mattonem commented 3 years ago

Btw. what is the difference between this and firstCompleteOf: ?

firstCompleteOf: will stop at any success or failure and return the success or failure...

Based on your comments on #104, I guess this is not the behaviour you would have expected. But changing the behaviour of firstCompleteOf: would be a major change as it is quite an old one...