search

Thomvis / BrightFutures

Write great asynchronous code in Swift using futures and promises
MIT License
1.9k stars 184 forks source link

Map by KeyPath #212

Closed RomanPodymov closed 4 years ago

RomanPodymov commented 4 years ago

Hello. Thank you for BrightFutures. In this pull request I added func map<U>(_ context: @escaping ExecutionContext, keyPath: KeyPath<Value.Value, U>) -> Future<U, Value.Error>. It is similar to map that you already have, but accepts KeyPath instead of a closure.

Thomvis commented 4 years ago

Hi, thanks for contributing to BrightFutures!

I like your contribution and I'd love to merge it after discussing one thought I had while going through the changes. Do you think you could leverage the closure-based map implementation to simplify the new map function? So instead of calling into onComplete, call map with a closure that uses the key path. That way we're guaranteed to match the behavior of map.

Let me know what you think!

RomanPodymov commented 4 years ago

Hello @Thomvis I agree with you. Also set !swift(>=5.2) for all new functions because Swift 5.2 provides Key Path Expressions as Functions. testMapByKeyPath is available for all Swift versions.

Thomvis commented 4 years ago

Thanks for making these changes. I've merged the PR! :)