Skip to content
This repository has been archived by the owner on Mar 16, 2020. It is now read-only.

Should flatMap be map? #15

Closed
b123400 opened this issue Nov 15, 2015 · 12 comments
Closed

Should flatMap be map? #15

b123400 opened this issue Nov 15, 2015 · 12 comments

Comments

@b123400
Copy link

b123400 commented Nov 15, 2015

flatMap should have a function signature of Signal a -> (a -> Signal b) -> Signal b, but it is currently Signal a -> (a -> b) -> Signal b which is a functor mapping, not flatMapping.

@JensRavens
Copy link
Owner

Which of the 3 implementations of flatMap do you mean?

flatMap of Signal<T> is just a nice wrapper around flatMap of Result<T>. So although a signal of a gets mapped to a signal of b, the underlying type of Result gets flat mapped. The name should mostly imply that map-functions are not allow to fail (they have to return a type) while flatMap-functions can return an error instead (resulting in a change from value to error of the parent type).

@b123400
Copy link
Author

b123400 commented Nov 16, 2015

Correct me if I am wrong, but I learned the idea of flatMap from Haskell, which is >>=.
The flatMapping function should expect a function that returns a signal, which means

flatMap<U> : ( T -> Signal<U> ) -> Signal <U>

Because Result<U> is not a Signal<U>, it is not flatMapping. To be accurate, the current flatMap function is flatting a Result, but mapping a Signal, which is not what a signal should be doing.

If you want to add error handling to signal, it should be a Signal<Result<T>>.
If you want a error stream, you can use filter to transfrom a Signal<Result<T>> to a Signal<T>.

The current implementation of Signal is actually 2 signals combined, one for Result.Success and one fore Result.Error

@JensRavens
Copy link
Owner

Yes, you're right: These are not signals from haskell, they're working a bit differently (e.g. they have internal state). Signal is just an async wrapper that makes it possible to execute a chain of A->Result<B> and B->Result<C>. There is no Signal without Result as they're tightly intertwined (opposed to haskell's signal). Kind of a box inside a box they're always working together.

If Signal would work without Result you would be 100% right - with the current state I'm not that sure as it is flattening something - just not itself.

@bencochran
Copy link

I think @b123400 is making the same general argument as Gordon Fontenot makes here: http://buildphase.fm/90 (regarding Array<T>’s func flatMap<U>(T -> U?) -> Array<U>)

Basically that flatMap as well-defined monadic operation, that on Monad<T> it would have the signature of (T -> Monad<U>) -> Monad<U>.

I think you can definitely decide to use other meanings of flatMap if you want (just as the Swift stdlib does), but wanted to give some reference to the “that’s not flatMap” argument.

@JensRavens
Copy link
Owner

I completely get your point. The only problem in renaming I see is that it's not map either: the current version gets the T of the Signal and returns a result of T, but the resulting Signal is of type T, not Result (Although you could argue that the signal holds a result internally of course).

Currently the implementations of signal and result are so much intertwined that it's not possible to use the signal without result. I'm thinking about a version 2 which splits them up - but I don't see that much value in a signal without error handling.

@JensRavens
Copy link
Owner

So in total: it's not map (wrong argument of the transform) and not flatMap (wrong result type). But flatMap gets closer to the intention of flattening the results.

@bencochran
Copy link

Yeah, that’s the tricky part. Though I think a consumer of the API shouldn’t be quite so concerned with the technical implementation details regarding Signal’s relation to Result. Not only does it expose unnecessary details to the consumer, but also couples the ideas of these two types together in a way that locks the implementation to a specific “shape” making it harder to change that in the future.

Back to the specific flatMap conversation, though, I see the following related operations (with thoughts as comments):

// This is definitely `map`
func map<U>(f: T -> U) -> Signal<U>

// I can see the argument for calling this `flatMap`,
// similar to Array<T>’s flatMap(T -> Optional<U>) -> Array<U>
// ReactiveCocoa calls this `attemptMap`
func flatMap<U>(f: T -> Result<U>) -> Signal<U>

// This feels like like `map` to me. I know under the hood it’s
// essentially the same as the previous, but it’s signature is
// basically just `T -> U`. In fact, with the current implementation, 
// you can pass the same (non-throwing) `T -> U` function to both
// `map` and this `flatMap`, which definitely feels weird.
//
// I vote remove this one and make `map` take `T throws -> U`, which
// lets you use it with either a throwing or non-throwing function.
// (Unless you want to introduce `attemptMap` in which case I *could*
// see keeping this as a version of that to keep it explicitly visible where
// errors can be introduced into a signal, which I actually like better
// personally).
func flatMap<U>(f: T throws -> U) -> Signal<U>

// I have no idea what this one is. It’s shape is more like a subscription/sink 
// than an operation.
func flatMap<U>(f: (T, (Result<U>->Void))->Void) -> Signal<U>

Conspicuously missing is a true flatMap such as func flatMap<U>(f: T -> Signal<U>) -> Signal<U>. Is there some reason for the omission?

@bencochran
Copy link

(Also, I’d be remiss not to point out that I’m by no means an authority on any of this. I’m only just starting to get a feel for these particular functional programming conventions. So please take my comments as simply sharing some ideas and observations, not strong/strict criticism. 😄)

@b123400
Copy link
Author

b123400 commented Nov 24, 2015

Thanks for the replies,, how about rename it to something like then?
I find it similar to Javascript's Promise, which has build-in error handling, and they use then name then instead of flatMap, it just check the result value's type, if it is a Promise then do flapMap, otherwise do map.

@JensRavens
Copy link
Owner

I like the idea of introducing then as a new method and reserving flatMap for functions returning a signal. As this is a breaking change it would be part of a 2.0-release. I have to be sure about naming it then to not break it again soon ;-)

I also thought about naming it transform because it applies a transforming function to a contained result.

In the end this should be created as an extension to signals containing a result, as this operation is only permitted on result-signals and there is no usecase of having a then instead of a map for value-based signals.

@irace
Copy link

irace commented Dec 9, 2015

Sounds awesome – being able to build more Signal-based APIs that can then be composed together would be a huge boon.

@nixterrimus nixterrimus mentioned this issue Jan 10, 2016
6 tasks
@JensRavens
Copy link
Owner

flat_map now finally does what it is supposed to do, the old functionality has been renamed to then.

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

No branches or pull requests

4 participants