2

How do I write the below code in a functional style? I want to chain the executions of methods which return a CompletableFuture (i.e. playRound returns a CompletableFuture<Void>)

CompletableFuture<Void> current = CompletableFuture.completedFuture(null);
for (int i = 1; i <= rounds; i++) {
    final int t = i;
    current = current.thenCompose(it -> playRound(t));
}

return current;
ernest_k
  • 44,416
  • 5
  • 53
  • 99
Hemant
  • 33
  • 6
  • Is `playRound` asynchronous? – ernest_k Aug 11 '19 at 10:13
  • You could make it more functional with Stream.reduce. Just create a Stream of numbers from 1 to rounds and reduce it. – Tesseract Aug 11 '19 at 10:20
  • what is the purpose of returning `Void CompletableFuture` always? – Ryuzaki L Aug 11 '19 at 10:25
  • Why are you using a `CompletableFuture` at all? The code, you’ve given, is entirely sequential and modelling a direct dependency chain. So it’s just a convoluted version of `for(int i = 1; i < rounds; i++) { playRound(i); } return playRound(rounds);` And even that can be simplified when you get rid of returning an always-completed-with-`null` future. – Holger Aug 14 '19 at 10:14
  • @ernest_k yes, the `playRound` is asynchronous. – Hemant Aug 15 '19 at 12:31
  • @Holger, the playRound internally calls asks for the player to move, which is can be a long waiting process, based on if the Player is a bot or a Web player. – Hemant Aug 15 '19 at 12:34
  • So what? You are still executing all steps in the same thread. As said in my previous comment, your code still does the same as an ordinary loop, just more complicated. It doesn’t improve anything. – Holger Aug 15 '19 at 13:11
  • @Holger not necessarily. The thread may be freed up for other things during `playRound`. – Druckles Aug 15 '19 at 14:12
  • Nope, the work done *within* `playRound` still is executed within the caller’s thread. Only what it does actually submit as asynchronous job, could be done in a different thread. But apparently, your actual code looks entirely different. In your answer, the method now returns a meaningful result (`GameResult`) rather than `Void`. So, when you also pass the result of a previous round into the invocation of the next round, it could become sensible code. It would still make more sense to create a synchronous method doing the actual work and submit one async job to make all invocations. – Holger Aug 15 '19 at 14:46
  • I had understood from the OP's description that they are waiting for the user's input. – Druckles Aug 15 '19 at 14:54
  • @Holger, when the player move is requested in the play round, the future can be returned by any thread which basically let's the current thread to be freed to do other things. Even if the function returned void, the above statement would be true(if I am not missing something basic :)). The way I want to model my game is it controls the exceution and not any external service which orchestrates the game. Your approach is also great but doesn't fit what I want to achieve. Thanks for the inputs :) – Hemant Aug 15 '19 at 22:38
  • When `playRound` does not return the result state, to be fed in on the next invocation, it implies that you have some kind of hidden global state and leave it to the caller, to ensure that the next invocation does not happen before the previous operation completed, so the safety of the global state is not in control of `playRound`, but at the same time, `playRound` initiates asynchronous operations manipulating it. Responsibilities should not be spread in this way… – Holger Aug 16 '19 at 06:57

2 Answers2

2

To illustrate what SpiderPig was getting at (and at the fear of not making it any more readable), you can use a stream of integers:

return IntStream.rangeClosed(1, rounds).boxed()
        .reduce(CompletableFuture.completedFuture(null), (previous, t) -> previous.thenCompose(x -> playRound(t)), (a, b) -> a.thenCombine(b, (x, y) -> y));

The Java Streams API wasn't designed for this sort of use case though (which is why reduce is the only option you get). If for whatever reason your stream is run in parallel, it won't work (that's when the combine method is called).

You can use a different library like vavr to do a fold instead:

return List.rangeClosed(1, rounds)
        .foldLeft(CompletableFuture.completedFuture(null), (previous, t) -> previous.thenCompose(x -> playRound(t)));

And if you already have vavr and are able to use a different type of return value, just use their Future:

return List.rangeClosed(1, rounds)
        .foldLeft(Future.successful(null), (previous, t) -> previous.flatMap(x -> playRound(t)));

See here for the differences between reduce and foldLeft.

Druckles
  • 3,161
  • 2
  • 41
  • 65
  • `reduce` is not a `foldLeft`. The specified function has to be associative or in other words, the Stream implementation is allowed to evaluate, e.g. `f(a, f(b, c))` instead of `f(f(a, b), c)`, which would cause the execution of game rounds in the wrong order. That’s what happens when the API doesn’t model the actual dependencies correctly (i.e. the results of previous game rounds are not passed to the subsequent rounds as parameters but via hidden state). – Holger Aug 15 '19 at 14:50
  • I'd chalk that up to limits in the Stream API. It's not theoretically sound but it still *probably* does what you want under all practical conditions. I'd still recommend the foldLeft. – Druckles Aug 15 '19 at 14:56
  • It will fail as soon as someone adds a `.parallel()` to it… – Holger Aug 15 '19 at 15:18
  • Yes it will ;-) – Druckles Aug 15 '19 at 15:26
  • @Holger I've updated the answer to take your comment into account. – Druckles Aug 16 '19 at 11:59
1

Figured a solution by creating a new function which takes the current round number as an argument.

private CompletableFuture<GameResult> play(final int round) {
    return playRound(round)
            .thenApply(roundCompleteHandler)
            .thenCompose(result -> {
                if (result.round == this.rounds) {
                    return result;
                } else {
                    return play(round + 1);
                }
            });
}

Hemant
  • 33
  • 6