Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smart Key Paths and Bindings #331

Closed
andersio opened this issue Apr 18, 2017 · 4 comments
Closed

Smart Key Paths and Bindings #331

andersio opened this issue Apr 18, 2017 · 4 comments
Labels
Milestone

Comments

@andersio
Copy link
Member

andersio commented Apr 18, 2017

With the accepted the Smart Key Paths for Swift 4, BindingTarget would be supercharged with the ability to access any properties and their subfields of objects and mutable properties without hand-written closures.

It could remove the needs of all binding targets in the reactive extensions. That said it depends on how it would be incorporate in our binding syntaxes.

  1. The most verbose approach would be:

    BindingTarget(object: control, for: \.isEnabled) <~ viewModel.isEnabled
    BindingTarget(object: view, for: \.label.text) <~ viewModel.title
  2. The verbose approach to reuse reactive would be:

    control.reactive.bindingTarget(\.isEnabled) <~ viewModel.isEnabled
    control.reactive.bindingTarget(\.label.text) <~ viewModel.title
  3. The old-ObjC macro-ish approach would be:

    r(control, \.isEnabled) <~ viewModel.isEnabled
    r(view, \.label.text) <~ viewModel.title

    (3) is a serious contender given its brevity as compared to (1). It can also replace .reactive as the gateway to the reactive extensions. It also does not suffer as much as .reactive from cross-module name conflicts, since at least it can still be disambiguated by the module name.

    The downside is that it violates the Swift API Guidelines, which state that free functions should be used only:

    1. When there’s no obvious self
    2. When the function is an unconstrained generic
    3. When function syntax is part of the established domain notation
  4. The approach to reuse reactive would be:

    // A `for:` label doesn't seem necessary here.
    control.reactive(\.isEnabled) <~ viewModel.isEnabled
    control.reactive(\.label.text) <~ viewModel.title
  5. An infix operator based approach would be:

    control .~ \.isEnabled <~ viewModel.isEnabled
    control .~ \.label.text <~ viewModel.title
  6. A prefix or postfix operator based approach would be:

    // The operator returns `(KeyPath<Base, U>) -> BindingTarget<U>`.
    .~control(\.isEnabled) <~ viewModel.isEnabled
    .~control(\.label.text) <~ viewModel.title

There are more combinations, but these are listed based on the requirement of having enough contextual information for autocompletion of key paths to show up in the LTR order.

I suppose most would prefer (4) or (5).

/cc @ReactiveCocoa/reactiveswift

@andersio andersio added this to the 3.0 milestone Apr 18, 2017
@andersio andersio changed the title Smart Key Paths and Reactive Smart Key Paths and Bindings Apr 18, 2017
@mdiep
Copy link
Contributor

mdiep commented Apr 18, 2017

The current approach seems simplest to me. But we should definitely play around with it once a beta is available

@andersio
Copy link
Member Author

andersio commented Apr 18, 2017

If you mean maintaining computed properties on Reactive as the current approach, it could read the simplest. But if we can have a generic solution that is close in legibility, it would considerably reduce our API surface.

The main drivers behind our current design (IIRC) were discoverability and type safety over KVC string key paths after all. Smart Key Path helps address the second, while the first one remains to be seen.

@jjoelson
Copy link
Contributor

Just jumping in here as a non-contributer: as someone who has gotten quite used to the current approach, (2) and (4) are very legible. If continuity is a priority then it makes sense to me to reuse reactive.

@andersio
Copy link
Member Author

Implemented by #440 and ReactiveCocoa/ReactiveCocoa#3489.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants