spotify / completable-futures

Utilities for working with futures in Java 8
Apache License 2.0
393 stars 51 forks source link

Add multi-compose #51

Closed eshrubs closed 7 years ago

eshrubs commented 7 years ago

This is similar to combine but works with async functions.

codecov-io commented 7 years ago

Codecov Report

Merging #51 into master will not change coverage. The diff coverage is 100%.

@@           Coverage Diff           @@
##             master    #51   +/-   ##
=======================================
  Coverage       100%   100%           
- Complexity        0     41   +41     
=======================================
  Files             4      4           
  Lines            64     92   +28     
  Branches          3      2    -1     
=======================================
+ Hits             64     92   +28
Impacted Files Coverage Δ Complexity Δ
...n/java/com/spotify/futures/CompletableFutures.java 100% <100%> (ø) 35 <8> (+35) :white_check_mark:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ef8b88f...8a9b024. Read the comment docs.

mbruggmann commented 7 years ago

@dflemstr @spkrka @pettermahlen Any strong opinions on this one? I think it's good!

eshrubs commented 7 years ago

@pettermahlen I agree with that sentiment. compose is like combineAsync. I kind of like sticking with the Java 8 compose terminology.

Do people have preferences for:

compose combineAsync composeMany

pettermahlen commented 7 years ago

@eshrubs Not sure I understand

I kind of like sticking with the Java 8 compose terminology.

I think that the Java 8 terminology for this is combine. compose composes/appends a function execution to a CompletionStage, whereas combine combines two stages. This PR introduces functionality that combines > 2 stages, so combine would be a better term, I think?

eshrubs commented 7 years ago

I can buy that!

I don't think combine(...., NaryFunction<...., CompletableFuture<R>>) would compile with the existing combine(...., NaryFunction<...., R>) so it will need a new name.

Some suggestions then could be: combineFuture composeMultiple

eshrubs commented 7 years ago

@pettermahlen I have updated the patch. Please comment :)

@spkrka @mbruggmann Please review the new terminology

eshrubs commented 7 years ago

Fixed!