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

R.is(Function) is not working correctly in R.ifElse #2654

Closed
Mati365 opened this issue Sep 19, 2018 · 6 comments
Closed

R.is(Function) is not working correctly in R.ifElse #2654

Mati365 opened this issue Sep 19, 2018 · 6 comments

Comments

@Mati365
Copy link

Mati365 commented Sep 19, 2018

How to reproduce:

const fn = R.ifElse(
    R.is(Function),
    R.identity,
    R.prop,
  );

fn('dupa')({dupa: 2}) // outputs: 2
fn(R.prop('dupa'))({dupa: 3}) // outputs: function

REPL: link
Did I do something wrong?

@nheisterkamp
Copy link

This seems like something that should work... The following do work as intended:

  • const fn = R.when(R.complement(R.is)(Function), R.prop)
  • const fn = f => R.is(Function, f) ? R.identity(f) : R.prop(f);

Although they all express the same implementation.

@Mati365
Copy link
Author

Mati365 commented Sep 19, 2018

Yep, but it is anyway weird

@CrossEye
Copy link
Member

The problem is that ifElse takes its arity from the longest of the three functions passed to it.

So when you pass it R.prop as the third argument, it returns a (curried) binary function. It sees prop as being (name, obj) => obj[name] and not the (name) => (obj) => obj[name] you would like. Ramda's prop can act in both ways due to Ramda's currying style, but it preserves the arity of the underlying binary function.

While you can fix this by wrapping R.unary around R.prop or using the simple lambda, (name) => (obj) => obj[name), I think either of these would be better:

const fn = when(is(String), prop)
const fn = unless(is(Function), prop)

For better or for worse, when and unless are not delegated to ifElse, and their APIs are a bit different. They both accept only unary functions. That's not a problem for prop, because of Ramda's means of currying.

This issue with ifElse has turned up a few times, but I don't see any satisfactory alternative. Here, it's being used to create a function with a complex API, so it doesn't bother me too much. Is this signature necessary? Accept a function and return it or accept a string and return the function generated by calling prop with that string -- that seems a bit hairy.

@stefano-b2c2
Copy link

stefano-b2c2 commented Sep 19, 2018

What's wrong with:

const fn = f => R.is(Function, f) ? f : R.prop(f);

It's less keystrokes and easier to understand? Also, though I agree with @CrossEye, this seems like an odd function. What are you trying to do?

Edit: Oops, just realised that @nheisterkamp made the same point.

@bouzlibop
Copy link
Contributor

This issue with ifElse has turned up a few times, but I don't see any satisfactory alternative.

@CrossEye could a note be added to ifElse docs? It might clarify it bit. Maybe something like this:

Note that ifElse takes its arity from the longest of the three functions passed to it.

@CrossEye
Copy link
Member

@bouzlibop: I have no objection. But I'm not sure how much it would help. I think the real problem is that it's easy to forget that something like R.prop has arity 2. I mostly use it as though it were the fully curried (name) => (obj) => obj[name].

customcommander added a commit to customcommander/ramda that referenced this issue Feb 6, 2022
customcommander added a commit to customcommander/ramda that referenced this issue Feb 7, 2022
customcommander added a commit to customcommander/ramda that referenced this issue Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants