Skip to content
This repository was archived by the owner on Jul 3, 2022. It is now read-only.

Map by KeyPath - #212

Merged
Thomvis merged 4 commits into
Thomvis:masterfrom
RomanPodymov:master
Feb 18, 2020
Merged

Thomvis merged 4 commits into
Thomvis:masterfrom
RomanPodymov:master

Conversation

@RomanPodymov

Copy link
Copy Markdown
Contributor

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

Thomvis commented Feb 16, 2020

Copy link
Copy Markdown
Owner

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

Copy link
Copy Markdown
Contributor Author

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
Thomvis merged commit 6ce6f31 into Thomvis:master Feb 18, 2020
@Thomvis

Thomvis commented Feb 18, 2020

Copy link
Copy Markdown
Owner

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants