Promisekit: `flatMap` expectations

[stephencelis]

• PromiseKit version: 6.0.0

I know we're in maintenance mode here, but I was wondering if you'd be open to a small discussion on the flatMap introductions in the most recent version, since Swift's long overloaded the term and made it more confusing than it needs to be (and again, recently, with compactMap), and I'm afraid the naming here may continue to bewilder folks both new to and familiar with functional programming.

The concepts of map and flatMap in programming at large have this shape:

// map
[A]  => ((A) ->   B ) ->  [B]
 A?  => ((A) ->   B ) ->   B?
F<A> => ((A) ->   B ) -> F<B>

// flatMap
[A]  => ((A) ->  [B]) ->  [B]
 A?  => ((A) ->   B?) ->   B?
M<A> => ((A) -> M<B>) -> M<B>

For PromiseKit to behave as one familiar these shapes expects, map and flatMap should look like this:

Promise<A> => ((A) ->         B ) -> Promise<B>
Promise<A> => ((A) -> Promise<B>) -> Promise<B>

These are the two behaviors of then! The Promises/A+ spec decided to combine map and flatMap behaviors for ergonomics. Overloads in JavaScript don't have the compiler error issues you've been trying to solve, but having predictable map and flatMap would help!

The flatMap additions to PromiseKit run counter to these intuitions. Maybe some qualifiers would help.

https://github.com/mxcl/PromiseKit/blob/0e150f87e895d849287e8fc5053e6dfe7919577d/Sources/Thenable.swift#L57

This looks like it's a compactMap of sorts, swallowing up optional return values.

https://github.com/mxcl/PromiseKit/blob/0e150f87e895d849287e8fc5053e6dfe7919577d/Sources/Thenable.swift#L176

This is performing a flatMap operation on the inner sequence, not the promise, so maybe flatMapInner would be a more appropriate name.

Lemme know if I've been unclear or need to make any clarifications!

[mxcl]

I'm copying flatMap from Swift’s Optional, which behaves this way. Also the Array form behaves this way especially now one version was renamed compactMap. So it seems to me that I'm being consistent with the Swift standard library, which is my intent and we'll change our names if we are wrong. It's not too late, though the deprecations would be ugly, but I'd rather be right for sure.

Optional<Data>.none.flatMap(UIImage.init)  // => nil
Data().flatMap(UIImage.init)  // => nil
validImageData.flatMap(UIImage.init)  // => UIImage

Optional<Int>.none.map{ $0 + 1 } // => nil
Optional.some(1).map{ $0 + 1 }  // => 2

[mxcl]

I didn't have time right this second to read your post thoroughly, but I will. Just thought I'd throw the above in in case it helped.

[stephencelis]

Ah, so Optional's flatMap works this way because it's defined on Optional itself. flatMap generally expects that the container type (where flatMap lives, _e.g._ Optional) is the same as the return type of the transform function (again, Optional in that case).

Optional<A> => ((A) -> Optional<B>) -> Optional<B>

So to define flatMap on Promise generically, the expectation is that the signature follows suit, where you can simply substitute every Optional for Promise.

Promise<A> => ((A) -> Promise<B>) -> Promise<B>

Array works the same way. (Sequence must convert the container to an Array because Swift's type system isn't quite powerful enough to return the same sequence type.)

Compare with compactMap, where it's not as abstract:

[A?] => ((A?) -> B?) -> [B]

These overloads on Promise are definitely handy! They're just not quite what folks expect when they see flatMap.

Hopefully this all makes sense! We're in abstract land, so it can be tough to convey this stuff. I'm hoping seeing the function signatures helps!

[mxcl]

OK, what do you suggest?

[stephencelis]

Just some renaming. Earlier I suggested the idea of sharing compactMap to mean failing on optional, and adopting flatMapInner to invoke flatMap on a sequence or optional that a promise contains.

What do you think?

[mxcl]

For me, the way to think about promises is to imagine they didn't exist, to abstract away the asynchronicity. Thus it's like you called flatMap on the promise's value and not the promise, so the functional names should be named as though they act on the values rather than the promises.

Of course this falls flat since the value is not an optional mostly. So really we should call it ifLet I guess, but this sucks too.

Anyway, I'm all for not confusing functional names more in any land, especially Swift land. So provided we can figure out good names, I'm down.

In this respect, is map also bad?

[stephencelis]

In this respect, is map also bad?

I don't think so! From what I gather map is being used as:

Promise<T> => ((T) -> U) -> Promise<U>

https://github.com/mxcl/PromiseKit/blob/0e150f87e895d849287e8fc5053e6dfe7919577d/Sources/Thenable.swift#L38

This is exactly what we want when we encounter map: it works with a transform function that has no knowledge of the container type, just the type contained within. Lemme know if there are other instances I missed, though, that have a different shape!

[stephencelis]

For me, the way to think about promises is to imagine they didn't exist, to abstract away the asynchronicity. Thus it's like you called flatMap on the promise's value and not the promise, so the functional names should be named as though they act on the values rather than the promises.

I hear ya! I just think this is a tough goal to achieve and it's what async/await constructs attempt to sugar over. Promise is a great solution but it lives squarely in the land of nested types, so I think we need to disambiguate between the container and the type within to avoid some confusion.

[stephencelis]

One other thought before I call it a night: there are some nice methods on observable/signal types in FRP libraries like ReactiveSwift and Rx that ignoreNil() or skipNil() to promote Signal<A?> to Signal<A>. If the goal is to easily transform a promise of A? to A and then act on that data, maybe some similar, concrete methods would be a nice alternative!

[mxcl]

Just brainstorming:

• mapIfSome
• mapIfNotNil

Or should we stick clear of map here?

[nathanhosselton]

tl;dr I think the feeling of the way PMK uses these functions is spot on, which IMO is at least as important as using them accurately.

I don't feel qualified to comment on the technical underpinnings of @stephencelis's reasoning here, which I'm sure is sound. I don't have any experience with functional programming, and my understanding of these transforming functions has come to me more through feeling than concrete expectations. I imagine I'm not the only PMK user in this boat though. So I'm gonna throw in my two cents, which admittedly is mostly opinion.

I understood the purpose of these new functions almost immediately. I did need to read the documentation on them first. What did it mean to map a promise? What did it mean to flatMap a promise? But as soon as I read the docs, I got it. Oh, maping a promise means I'm returning something that's not a promise. It's like a transform. Got it. Oh, flatMaping a promise means I'm returning an Optional and I want to remove that optionality. Just like how I (most often) use flatMap in Swift. Got it.

These understandings were based on the feeling of my experience with the functions they're based on, not the technical expectations, which I've never had to think about. A lot of people do think about these things of course, and I can understand how our co-opting of the functions might be confusing due to that. So if some better names are found, then cool. But I do worry about adding qualifiers to the names (e.g. ifSome, ifNotNil), or changing the names entirely for the simple purpose of disassociating them from the pure counterparts. I worry it'll actually make the purpose less clear. I believe my quick understanding of their purpose came from the simple naming that (at least somewhat closely) mirrored behavior I was already familiar with in Swift.

[stephencelis]

@nathanhosselton map is being used in the expected way! If follows in the footsteps of Array.map and Optional.map in Swift, and it follows in the footsteps of map in a lot of other programming languages, and in math, and in other disciplines! This is great! It means we can go between these different worlds and share this knowledge without having to start from scratch each time. As soon as we write map in a different way, we start to lose this, and we lose the trust of people who have built up these intuitions and expectations.

This is exactly why flatMap should do the same: it also has a very specific, established meaning over a _lot_ of different places. Even Swift has cleaned house here: Array.flatMap only works with a closure that returns an array, and Optional.flatMap only works with a closure that returns an optional. (If you're used to using Array.flatMap with optionals, you'll be renaming those to compactMap soon.)

The current implementations in PromiseKit unfortunately deviate from the long history of what flatMap is and muddies the waters for functional and non-functional programmers alike! We need to be careful not to overload something with an established meaning (even if it's abstract) with something that doesn't match that meaning. Even though it kinda looks like a duck and it quacks like a duck, it's just not a duck. If we mistake a horse for a dog because we've seen some dogs but have never seen a horse, we're not going to keep on calling horses "dogs" when we learn about horses. And we're not going to reuse + for multiplication when we first learn multiplication because + already has a clear meaning, and overloading it (instead of establishing another operator to host this shared meaning, *) confuses things.

I think the constructive way to continue the conversation here is to suggest new names that feel right and don't overload concepts that have well-established meaning.

[nathanhosselton]

Apologies, restating information here so I am sure I'm on the same page.

The issue with our flatMap that operates on Optionals is that it is called on a promise, and this is mismatched with the transform return type, which is an Optional. A true flatMap's transform return type matches the type on which the function is called.

The issue with our flatMap that operates on Sequences is essentially the same: it is called on a promise, but the promise is ignored in the transformation; it's the sequence type within the promise that is actually flatMaped.

Is this right?

[stephencelis]

@nathanhosselton I'm not exactly clear as to what you're asking, but none of the flatMap overloads on Promise fit the shape that flatMap expects:

(M<A>) => ((A) -> M<B>) -> M<B>

It operates on a single container type, M, and works to transform Ms that contain As into Ms that contain Bs with a specific transform function from A to M<B>.

I think it's important to realize that flatMap is a completely generic concept and doesn't know anything about Array or Optional. It just turns out that you can satisfy the flatMap function signature for both types by swapping M out, and it also turns out that these two functions are useful. You can also swap M with Promise and get something useful (the thing that then does, where you can return a promise from the closure). You can swap M out for a _lot_ of things.

[nathanhosselton]

I'm just trying to restate the specific issue(s) with our flatMaps as succinctly as possible. But I want to confirm with you that I have it right, since this is my first time considering such things.

As you said, the shape of a pure flatMap looks like:

(M<A>) => ((A) -> M<B>) -> M<B>

And the shape of our flatMaps actually look like:

//Optional Thenable.flatMap
(Promise<A>) => ((A) -> Optional<B>) -> Promise<B>
//or
(M<A>) => ((A) -> N<B>) -> M<B>

//Sequence Thenable.flatMap
(Promise<Sequence<A>>) => ((A) -> Sequence<B>) -> Promise<Sequence<B>>
//or
(M<A>) => ((A) -> B) -> M<B>

In the case of our flatMap on Sequences, the shape becomes correct if the promise containers are ignored, which is why you suggested the name flatMapInner.

In the case of our flatMap on Optionals, the shape is still incorrect even when the promise containers are ignored:

(A) => ((A) -> N<B>) -> B

…which is why @mxcl mentioned that the name ifLet would actually be more accurate.

Do I have this right?

[stephencelis]

I think you have this right!

In the case of our flatMap on Optionals, the shape is still incorrect even when the promise containers are ignored:

(A) => ((A) -> N<B>) -> B

In this case, no need to abstract with N! We have a concrete, failover operation that utilizes Optional. This is why I suggested compactMap over ifLet since it would provide a shape consistent with Array.compactMap:

Array<A>   => ((A) -> Optional<B>) -> Array<B>
Promise<A> => ((A) -> Optional<B>) -> Promise<B>

If we settled on this, we'd have to rename the existing overload from compactMap to compactMapInner, but we'd have a consistent set of methods if we adopted flatMapInner for the other two operations.

[nathanhosselton]

What about flatMapValue or flatMapResult? And compactMapValue/compactMapResult for our flatMap variant that removes Optionals from a Sequence?

Value/Result of course refer to the promise's internal value, so we're being explicit about what is actually being flatMapped while using PMK-familiar terminology.

[stephencelis]

Either sound good to me!

[mxcl]

Yeah, so really then is flatMap, which I kinda already knew.

K, well I don't want to muddy anything here, let’s rename both flatMaps, the non-sequence one can have a nice name we can invent, the latter should probably be flatMapInner though I think this maybe this just confuses things (or maybe it's the opposite?). Still it's ugly. But IME it is rarely used.

Mainly my concern is a sensible set of names that people understand without having to read the documentation. Then after that is that we don’t contribute to a wider misunderstanding of functional principles, and since PromiseKit is a popular library, we have that responsibility.

Thanks for the input, let's keep it going.

[mxcl]

firstly {
    fetchImageData()
}.ifLet {
    UIImage(data: $0)
}

Clear, but ugly. Probably should be our choice, but boy…

[mxcl]

firstly {
    fetchImageData()
}.unwrap {
    UIImage(data: $0)
}

[JustinJiaDev]

After reading your conversation I feel that compactMapValues, compactMap, flatMapValues and flatMap make most sense to me. And it will follow Swift’s naming convention (https://developer.apple.com/documentation/swift/dictionary/2894692-mapvalues).

[mxcl]

So we have:

• Promise<Sequence>.flatMap ~> Promise<Sequence>.flatMapValues
• Promise<Sequence>.compactMap ~> Promise<Sequence>.compactMapValues
• Promise<T>.flatMap ~> Promise<T>.unwrap

How are we feeling about these? Additionally, how about filter which we also provide? Finally, do we feel that if we go to this length the Swift stdlib team will also?

[mxcl]

Could unwrap be compactMapValue and should it be? At this point I'm feeling a little hazy on understanding again.

[stephencelis]

Could unwrap be compactMapValue and should it be? At this point I'm feeling a little hazy on understanding again.

I think unwrap could just be plain old compactMap? It matches the overall shape of Array's compactMap but with Promise substituted in.

Array<A>   => ((A) -> Optional<B>) -> Array<B>
Promise<A> => ((A) -> Optional<B>) -> Promise<B>

Then compactMapValue would be for this case:

Promise<Array<A>> => ((A) -> Optional<B>) -> Promise<Array<B>>

Additionally, how about filter which we also provide?

Hm, interesting. So if I came across promise.filter, I'd maybe think that it was filtering a promise by the value it contains, not filtering the sequence a promise contains. Does such a thing live on promise under a different name? One nice thing is that a filter that works this way can be built with compactMap:

func filter<A>(_ predicate: (A) -> Bool) -> Promise<A> {
  return self.compactMap { predicate($0) ? $0 : nil }
} 

[mxcl]

Please review this preliminary PR: #782

[mxcl]

Merged for release with 6.1.0 (today hopefully).