vapor / core

🌎 Utility package containing tools for byte manipulation, Codable, OS APIs, and debugging.
MIT License
82 stars 50 forks source link

Replace `Future.flatMap` with `Future.then`? #183

Open MrMage opened 6 years ago

MrMage commented 6 years ago

Currently, Future.flatMap is implemented like this: https://github.com/vapor/core/blob/fda3ebda8046af7c9a886fa4a5cfd886111ac23e/Sources/Async/Future%2BMap.swift#L45

Would it be possible to call through to SwiftNIO's implementation of Future.then instead?

https://github.com/apple/swift-nio/blob/6f3671de4ed1350b86762e0da317075f69983f7d/Sources/NIO/EventLoopFuture.swift#L437

Let me know if I'm missing something.

vzsg commented 6 years ago

It's certainly possible, all you need to mind is that flatMap allows the closure to throw, while then does not.

This should be equivalent to the current implementation:

extension Future {
    func flatmap2<T>(to type: T.Type = T.self, _ callback: @escaping (Expectation) throws -> Future<T>) -> Future<T> {
        return self.then { [eventLoop = self.eventLoop] expectation in
            do {
                return try callback(expectation)
            } catch {
                return eventLoop.newFailedFuture(error: error)
            }
        }
    }
}
MrMage commented 6 years ago

Thanks! I did not spot that then does not take a throwing closure. Leaving this open in case we want to change the implementation, e.g. for Vapor 4 (although this should be a non-breaking change anyway).