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

Add Signal and SignalProducer operations as methods #1941

Closed
ColinEberhardt opened this issue Apr 26, 2015 · 23 comments
Closed

Add Signal and SignalProducer operations as methods #1941

ColinEberhardt opened this issue Apr 26, 2015 · 23 comments
Milestone

Comments

@ColinEberhardt
Copy link

I've just been digging in to RC3.0, and in general y I really like it. There's certainly some clever stuff going on in there - although some of it feels too clever to me!

Learning about the pipe forward and its usage with the curried free functions was a lot of fun. However, I do wonder whether this will make the RC3.0 APIs less accessible?

It is less discoverable, you have to hunt out the operations that can be applied to a signal, rather than having them presented to you as part of the signal's API. This is worse for signal producer where |> is also used to apply signal operations to signal producers, that one is probably going to baffle a lot of people!

I think it would make the API easier to use if you also added the signal and signal producer operations as methods directly on these types. This mirrors the swift API for collections, where they are available as free functions that are defined for the collection protocols, but are also present as methods directly on the concrete types.

@NachoSoto
Copy link
Member

You are completely right: making them methods would make them easier to use.

Unfortunately that is not possible right now because Swift doesn't allow methods to be constrained further than the class itself. For example, an operator like skipRepeats wouldn't be possible because it can only be defined for T: Equatable, but Signal doesn't enforce that.

@ColinEberhardt
Copy link
Author

That's a very good point, I hadn't noticed that some operations provided their own constraints.

I guess in those instances it could be a runtime error? (Yuck)

@neilpa
Copy link
Member

neilpa commented Apr 26, 2015

Lack of method constraints isn't an issue. Methods can offer an explicit closure param that satisfies what the constraint implies. This is already the case for skipRepeats. The Equatable overload is a wrapper for signal |> skipRepeats { $0 == $1 }

Offering an explicit closure is also more flexible since it doesn't require you to use the default comparators when you want different behavior. Of course you can still provide a convenience free function with the constraints.

@NachoSoto
Copy link
Member

@neilpa how would you define that method though? You'd have to specify a new generic parameter T, different than the class'.

@neilpa
Copy link
Member

neilpa commented Apr 26, 2015

The generic is the same, you just have to explicitly provide an isRepeat function over T.

extension Signal {
    public func skipRepeats(isRepeat: (T, T) -> Bool) -> Signal {
        return self
            |> map { Optional($0) }
            |> combinePrevious(nil)
            |> filter {
                switch $0 {
                case let (.Some(a), .Some(b)) where isRepeat(a, b):
                    return false
                default:
                    return true
                }
            }
            |> map { $0.1! }
    }
}

@neilpa
Copy link
Member

neilpa commented Apr 26, 2015

Though some things like ignoreNils couldn't be defined directly. However, you could still achieve it via a more generic filterMap.

extension Signal {
    public func filterMap<U>(transform: T -> U?) -> Signal<U, E> {
        return self |> map(transform) |> filter { $0 != nil } |> map { $0! }
    }
}

signal.filterMap { $0 }

@ikesyo
Copy link
Member

ikesyo commented Apr 27, 2015

Signal.dematerialize and SignalProducer.flatten would require weird evidence parameter and passing identity function to it. 😞

@neilpa
Copy link
Member

neilpa commented Apr 27, 2015

I'm not suggesting all operators be migrated to methods, just that a number of them could without too much type-checker gymnastics. Restricting it to just a few common operators like map and filter would aid those getting acquainted with the API via auto-complete.

@kastiglione
Copy link
Member

What of consistency? Having some operators as methods, some as functions, sounds worse to me than having |> instead of .. Same for having different signatures depending on whether the operator is a function or method.

@neilpa
Copy link
Member

neilpa commented Apr 27, 2015

Xcode is a lot worse at autocomplete when it comes to piped free functions vs methods.

My vote would be for all the operators to remain as free functions as they are today. However, additionally expose a subset as methods where it doesn't require jumping through hoops to workaround type-checker limitations.

@texastoland
Copy link

Tool support is a disappointing argument to me relative to concise and consistent API. Tooling will follow the way code gets written. I'll report an issue on YouTrack to start a discussion for Hackage-like completion in AppCode. I imagine a community plugin wouldn't be impossible in Xcode.

@neilpa
Copy link
Member

neilpa commented Apr 28, 2015

Tooling will follow the way code gets written

I wouldn't hold my breath when it comes to Apple and Xcode.

@texastoland
Copy link

I wouldn't hold my breath when it comes to Apple and Xcode.

I spend as little time in Xcode as possible but have been impressed by recent community contributions.

I tweeted AppCode till my YouTrack account gets sorted.

@ColinEberhardt
Copy link
Author

What of consistency? Having some operators as methods, some as functions, sounds worse to me

It is consistent with the way that the Swift APIs present themselves, e.g. there is a global map function, but also a map method defined directly on Array.

Tool support is a disappointing argument to me relative to concise and consistent API

I can see your point, and agree that tool support will improve, whether through Apple / Xcode itself, or through the community or AppCode.

However tooling is only part of my argument. The other issue is the overall complexity of this approach.

The pipe forward operator is a pretty advanced concept, especially the overloaded usage that allows you to apply signal operations to signal producers. Critically, if you do not understand how this works, it is far from obvious how to apply a simple map operation to a signal producer.

My vote would be for all the operators to remain as free functions as they are today. However, additionally expose a subset as methods where it doesn't require jumping through hoops to workaround type-checker limitations.

👍 to this!

@NachoSoto
Copy link
Member

is consistent with the way that the Swift APIs present themselves, e.g. there is a global map function, but also a map method defined directly on Array.

They're not exactly the same function though. The free functions operate on more abstract types, like CollectionType, in contrast to the similar method in Array.

@ColinEberhardt
Copy link
Author

is consistent with the way that the Swift APIs present themselves, e.g. there is a global map function, but also a map method defined directly on Array.

They're not exactly the same function though. The free functions operate on more abstract types, like CollectionType, in contrast to the similar method in Array.

Very true, but I believe the underlying motivation is the same.

@texastoland
Copy link

Very true, but I believe the underlying motivation is the same.

I quite see your point as well. The methods act like a gateway to the free functions if you will.

some operators as methods, some as functions, sounds worse to me than having |> instead of ..

Perhaps a separate module like ReactiveCocoaEx to opt in extension methods on Signal[Producer]?

But I feel like pipe is easier to grok than FRP though. The core concern sounds like a case of incidental (possible functional dogma) versus essential (FRP is Functional™️) complexity.

@nomothetis
Copy link

For my part, I think the complexity of |> is relatively unimportant. It works naturally in the context of what needs to be understood:

let signal = baseSignal
    |> map(newElementFromElement)
    |> tryMap(newElementOrErrorFromElement)
    |> mapError(outputErrorFromError)

let signalProducer = baseProducer
    |> map(newElementFromElement)
    |> tryMap(newElementOrErrorFromElement)
    |> mapError(outputErrorFromError)

It doesn't matter that the operator does different things; the code is still clear. When users start to create their own primitive operations, it will become important for them to understand the difference, but I'm not sure how much it matters until then, and I think by that point they will have the understanding required to take the next step.

@jspahrsummers
Copy link
Member

I think this is a classic Simple vs. Easy thing. It's certainly easier (more familiar, approachable) to find operations as methods, but we run aground of all the complexity that folks have already alluded to in this thread (constrained types, duplicate implementations).

I'd rather keep the API simpler and define operations consistently in one way. It might involve a steeper learning curve, but it won't involve learning the rules and exceptions of when an operator is also available as a method (or how to safely use it as such).

I'll also note that |> is partly unintuitive because there are basically no docs. The 3.0 API is documented only in “header” comments, and adding real, high-level documentation and introductory materials should largely solve this problem. Only then can we truly evaluate what is understandable and what isn't.

@jspahrsummers jspahrsummers added this to the 3.0 milestone May 3, 2015
@ColinEberhardt
Copy link
Author

Thanks everyone for considering this. @jspahrsummers - I agree with your point that with RC3 having minimal documentation it is probably too early to say whether it is understandable or not.

@prefect42
Copy link

With Swift 2.0 and the new protocol extensions this seems to be possible now…

@rhysforyou
Copy link
Contributor

@prefect42 yep, it's under discussion in #2087

@ColinEberhardt
Copy link
Author

Cool, really glad to see my suggestion being revisited thanks to Swift 2 features.

On a separate note, they ever-changing swift language does not make it easy for framework / library authors!

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

No branches or pull requests

10 participants