Closed briancavalier closed 7 years ago
Merge this before or after #20?
this looks really neat :)
Hmmm, so I think this is actually backwards. Currently, bimap is implemented like bimap(mapFulfilled, mapRejected)
, such that the "left" function is applied to a fulfilled, i.e. successful, value. That's reversed from the typical convention around an Either, for example, where the right value is typically considered to represent "success".
That raises an interesting potential disparity: We have to choose whether the argument ordering of bimap should be more like bimap for an Either, or should be more like then. I'm inclined to go with Either's argument ordering since bimap is following Fantasy-Land, and then is following ES.
As another data point, Fluture uses the Either argument ordering.
LOL, and funnily enough, I wrote the bimap docs to use Either-ordering, even though the code used then-ordering.
Ok, I'm convinced ... going with Either-ordering.
At the expense of everyone's confusion, we also could do the same thing as for ap
- make .bimap
work like .then
, but let fantasyland/bimap
work like Either
.
Whenever I'm using methods, I'm in an OOP mindset and compare the call to then
;
when I'm using function (Ramda.bimap
) I'm functionally minded and think of the Either-like Promise type, where the first argument obviously has to map over the error (because I expect to be able to partially apply it).
me personally if i see bimap
method on a promise i personally expect it to work like Either
each and every time... in contrast if I am coming from a different background of not FP
then i would probably not make any assumptions about how the bimap
works and just go with the API implementation however if I for some reason go to another library such as Task or a Future
I would expect the API to match if they both implement the same method.
Again these are just my personal takes on the subject.
However I think we can all agree that promises are just async Eithers
😛
make .bimap work like .then, but let fantasyland/bimap work like Either
It's certainly worth considering.
My thinking right now is that, since we don't have a backward compatibility problem in this case, I prefer calling then
the "outlier" and stick with the "typical" FP arg ordering. Bimap is already different from then
, so trying to make it more similar may actually cause more confusion. It's really hard to say for sure what'll happen tho :)
Maybe that lack of clarity is a reason just to pick something and do it, calling it v1.1.0, seeing how people react, and adjusting course for v2.0.0 as necessary.
Whenever I'm using methods, I'm in an OOP mindset and compare the call to then; when I'm using function (Ramda.bimap) I'm functionally minded
Yeah, I agree this is the essence of the problem. People who have learned about ES or A+ promises first will know then
and will have formed expectations around argument ordering from that. People who are coming to creed with an FP background will know Either (and the like) and will have formed expectations around argument ordering from that.
another library such as Task or a Future I would expect the API to match
That's pretty compelling, imho.
Another potential course is simply not to provide Promise.prototype.bimap
at all, and provide only Promise.prototype[fl.bimap]
. That seems more radical, although I wonder if anyone will actually use Promise.prototype.bimap
?
The major use case I can see for .bimap
is mapping over errors, without needing to throw …
or return reject(…)
in a .catch
handler. The other callback (for the success case) isn't that interesting.
Which leads to the thought… why not just provide a mapError
method only?
Calling .bimap(onerror, onsuccess)
is exactly equivalent to .map(onsuccess).mapError(onerror)
or .mapError(onerror).map(onsuccess)
, just with more confusion about argument order (but also one promise and one handler less instantiated, so a little more efficient).
Combine that with a [fl.bimap]
method and everyone should be happy.
How about this: let's decouple the decision of how to expose error mapping functionality from the decision to add FL 2.0 bifunctor. That means merging #86 first, then rebasing this PR and changing it to provide only the namespaced bimap. Then, we can continue to discuss the best first-class API(s) for bimap, mapError, etc.
Finally unwinding the PR stack and getting back to this. Just rebased, and will prune this back to just implementing the fantasy-land namespaced bimap shortly.
Calling .bimap(onerror, onsuccess) is exactly equivalent to .map(onsuccess).mapError(onerror)
They aren't exactly equivalent: if onsuccess
throws, onerror
will be called in the case of .map(onsuccess).mapError(onerror)
, but not in the case of bimap(onerror, onsuccess)
. It's true in the reverse order, .mapError(onerror).map(onsuccess)
, which is interesting because onerror
appears first in that and in bimap
.
I'm not sure if or how that helps us in making a decision about mapError
and public bimap
, tho.
Ok, 67c072a includes only a FL 2.0 namespaced bimap. The travis build fails because of jsinspect, annoyingly. If I bump up the threshold by 1 to 36, it passes :/
My mood right now on bimap
is that it's not harmful to expose it "publicly", i.e. non-namespaced, as long as we take care to document it as clearly as we can, with good examples. That also certainly doesn't rule out an additional mapError
.
Here's a first go. There are several ways to do it. As two examples, we could:
I went with 2 initially, but definitely open to discussion.
Todo
Close #33