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

Functor broken #1

Closed
puffnfresh opened this issue May 23, 2013 · 8 comments
Closed

Functor broken #1

puffnfresh opened this issue May 23, 2013 · 8 comments
Assignees

Comments

@puffnfresh
Copy link

  • u.map(function(a) { return a; })) is equivalent to u (identity)

Breaks for this example:

p.concat(p2).concat(p3).map(function(a) { return a; });
@mudge
Copy link
Owner

mudge commented May 23, 2013

This is a tricky one: at the moment, promises with multiple values will yield them as separate arguments to map instead of together as an array. This was purely because there is no elegant destructuring syntax in node yet. However, you're right that this breaks identity as defined.

I can change the behaviour of map to concatenate promise values into an array that is returned as a single argument during map.

@puffnfresh
Copy link
Author

You shouldn't make map always take an array. That's not a restriction that Fantasy Land imposes. Fantasy Land's is much more relaxed:

map :: f a -> (a -> b) -> f b

(i.e. you should be able to pass any function that takes an a and returns a b)

The best option I can see is making map run f over each of the resolved values.

@ghost ghost assigned mudge May 24, 2013
@mudge
Copy link
Owner

mudge commented May 24, 2013

I've been experimenting with this a little more; an issue is what it means to concat two Promises. At the moment, I have it like so:

concat :: Promise a -> Promise b -> Promise [a b]

But this means that, strictly, speaking, concating several promises in a row results in:

p.concat(p2) //=> Promise [a b]
p.concat(p2).concat(p3) //=> Promise [[a b] c]
p.concat(p2).concat(p3).concat(p4) //=> Promise [[[a b] c] d]

While consistent, my original use case was to have a way to combine multiple promises into one that only yields when all values are resolved. You would then need to make those values available and this interface was very nice to use:

promisedHttpRequest.concat(anotherPromisedHttpRequest).map(function (response, anotherResponse) {
    // Do something with the two response bodies.
});

As you say, this violates the Functor interface so I wanted to change it to be compliant:

promisedHttpRequest.concat(anotherPromisedHttpRequest).map(function (responses) {
    var response = responses[0],
        anotherResponse = responses[1];

    // Do something with the two response bodies.
});

However, the current definition of concat makes this surprising beyond two promises:

promisedHttpRequest.concat(anotherPromisedHttpRequest).concat(yetAnotherPromisedHttpRequest).map(function (responses) {
    var response = responses[0][0],
        anotherResponse = responses[0][1],
        yetAnotherResponse = response[1];

    // Do something with the two response bodies.
});

Perhaps defining the result of concat as strictly Promise [a b] is wrong as it would be nice have (Promise a) concat (Promise b) concat (Promise c) -> Promise a b c or perhaps I am abusing map in my above examples?

@puffnfresh
Copy link
Author

Ah, concat is broken. It should like this:

concat :: Promise a -> Promise a -> Promise a

@mudge
Copy link
Owner

mudge commented May 24, 2013

So it's only possible to concat things that can themselves be concatenated (e.g. arrays, etc.)?

@puffnfresh
Copy link
Author

@mudge that's one solution, or you could say that Promises always represent multiple values (of the same type) and then concat them internally.

So Promise could be something like an async Array, I guess.

@mudge
Copy link
Owner

mudge commented May 24, 2013

I like the idea of Promises as a kind of Eventual type: they might not have
a value now but they will eventually. If I define concat in terms of
concatenating contents (so only supporting promises of semigroups) then I
could build the interface I wanted with map on top of those primitives.

@mudge mudge closed this as completed in 1fbbe8b May 24, 2013
@mudge
Copy link
Owner

mudge commented May 24, 2013

I've just pushed some changes to the interface to bring it back in line with the specification, please feel free to review and let me know if there are any issues.

I added a test explicitly for the case mentioned (identity of a semigroup): https://github.com/mudge/pacta/blob/master/test/pacta_test.js#L131-L142

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

No branches or pull requests

2 participants