Consider using @autoclosure for EventLoopFuture.transform with future

[siemensikkema]

The current signature for the transform function on EventLoopFuture that takes a future is:

public func transform<T>(to future: EventLoopFuture<T>) -> EventLoopFuture<T>

And can be used as follows:

futureA.transform(to: makeFutureB())

To me, this reads as future b starting the work after future a has finished. However, this is not the case since future b will be created "synchronously" and therefore the work will start immediately. This could be prevented by changing the implementation of transform to:

public func transform<T>(to future: @escaping @autoclosure () -> EventLoopFuture<T>) -> EventLoopFuture<T> {
    return self.flatMap { _ in
        future()
    }
}

Are there any drawbacks I'm not seeing? Is this considered a breaking change?

[calebkleveter]

You have a good point, but I personally think that the current implementation should be kept. If someone wants to explicitly only start the next async operation if the previous one succeeds, they can use .flatMap instead. However, a note in the .transform docs would be a good thing to add so people understand what's going on.

[mattpolzin]

The way transform() works bit me in the beginning as well. Some added documentation might be helpful. Although it is actually nice in the end that transform does not wait for the future it transforms to finish (because it has no explicit dependency on the output of that original future), I personally avoid it to avoid the possibility of confusion over how my code will behave.

For some reason, and() never confused me the same way, so I guess it’s not just a matter of the name not implying whether waiting will occur.

[siemensikkema]

Thanks for your input. I think I'm now convinced it should stay the way it is because with @autoclosure the only way to ensure that the second future starts its work as soon as possible would be to assign it to a variable which is quite a hassle, whereas using flatMap over transform is only a minor inconvenience. As for the change in API documentation, we could add something like:

NB. transform does not wait for the current future to finish before starting work on the new future. If you need this behavior, use flatMap { _ in ... } and create your future inside its closure.