Replace Collection<Future>.flatten with Future.reduce?

[MrMage]

At the moment we are implementing `flatten ourself in

core/Sources/Async/Future+Flatten.swift

Line 42 in 3b72f2c

public func flatten(on worker: Worker) -> Future<[Element.Expectation]> {

.

We could investigate whether we could replace that with an almost-trivial call to https://github.com/apple/swift-nio/blob/82a6e4d8215660005127547106aa8cf10817cb78/Sources/NIO/EventLoopFuture.swift#L915.

[tanner0101]

As long as we can ensure the ordering remains the same as the current method, I'd be happy to do this 👍

[MrMage]

I think it should stay the same. Actually, I thought we had a test that flatten() created an array with the results in the correct order, but can't find it right now :-/

[jdmcd]

@MrMage looks like the second test in this file checks for the correct order https://github.com/vapor/core/blob/master/Tests/AsyncTests/AsyncTests.swift

[MrMage]

Good find! However, I think to properly test this the order of succeeding would need to be reversed:

    b.succeed(result: "b")
    a.succeed(result: "a")

[jdmcd]

My understanding is that flatten can succeed futures in any order as long as they are returned in the correct order. If I’m correct in that assumption, I think the test does what it needs to (it could also probably test succeeding in a different order and getting the same array result)

[MrMage]

At the moment, the test only ensures the correct order if the input futures already succeed in the same order, which should be trivial. Swapping the succeed calla would actially test that the final order is still correct, even if the futures succeed in the "wrong" order.

[jdmcd]

I see. Yeah that should be added

[MrMage]

Closing this because of #199.