Should flatMap be map? about interstellar HOT 12 CLOSED

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).

from interstellar.

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

from interstellar.

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.

from interstellar.

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.

from interstellar.

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.

from interstellar.

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.

from interstellar.

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?

from interstellar.

(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. 😄)

from interstellar.

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.

from interstellar.

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.

from interstellar.

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

from interstellar.

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

from interstellar.