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

Curry === is problematic #13

Closed
katywings opened this issue Jul 11, 2017 · 3 comments
Closed

Curry === is problematic #13

katywings opened this issue Jul 11, 2017 · 3 comments
Labels

Comments

@katywings
Copy link

First off, i like your work! Its really cool to learn fp style without reading through dozens lines of code :).

I know you copied the curry function from a gist comment 😄 , i took some time to learn how it works and notices a problem - especially when its used together with reduce

Problem in one sentence: If the curried function is called with too many arguments, it will return a new curried function instead of calling the final function. Also the new curried function will now return again a new curried function for every further attempt.
https://github.com/selfrefactor/rambda/blob/master/modules/curry.js#L4

...
o.length === f.length
...

A basic example (ES6):

const add = curry((n, n2) => n + n2
add(1, 2, 3)
// result: function
// desired: 3

Reduce example:

const add = curry((n, n2) => n + n2)
reduce(add, 0, [1, 2, 3])
// result: function

Reduce calls the add function not only with acc and value, but also with index and array
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/Reduce

  • One way to fix it, would be to change o.length === f.length to o.length >= f.length
  • An alternative would be to change the reduce function in the way, that it only calls with acc and value, (Ramda does it like that: http://ramdajs.com/0.23.0/docs/#reduce)
@selfrefactor
Copy link
Owner

I appreciate your nice words, but mostly catching these strange behaviors.

Back to business - I have edited curry upon your suggestion. and both of the above examples are included in the tests.

Just want to ask you, is there still a need for a change in reduce in the context that your reduce example now is working correctly?

Sorry, but I fail to fully understand the issue with reduce and I'll be grateful, if you help me out.

@katywings
Copy link
Author

Thanks for the fast feedback and changes!

The approach to change reduce was just thought as an alternative - rather bad alternative because the callback of builtin map also gets idx and array. There is no bug or so in reduce! Actually I noticed the bug in curry because I was trying around with reduce and a curried function.

In Ramda functions like reduce and map dont provide idx and array to the cb, actually they had a long discussion about this: ramda/ramda#452

In my opinion both ways (with and without idx and array) are perfectly fine, the library author should decide which one is the right ;).

@selfrefactor
Copy link
Owner

Thank you for the detailed explanation.

I think I will go with the current state and leave it as it is.

I am closing the issue now.

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

No branches or pull requests

2 participants