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

flip doesn't work when arity > 2 #464

Closed
romgrk opened this issue May 28, 2020 · 4 comments · Fixed by #469
Closed

flip doesn't work when arity > 2 #464

romgrk opened this issue May 28, 2020 · 4 comments · Fixed by #469

Comments

@romgrk
Copy link
Contributor

romgrk commented May 28, 2020

This is a difference with ramda and should either be documented as such or be made compatible.

If it's not made compatible, a more reasonable behavior than return undefined should be used, eg throw new Error("flip doesn't work with arity > 2").

rambda/src/flip.js

Lines 1 to 11 in f8f3895

function flipExport(fn){
return (...input) => {
if (input.length === 1){
return holder => fn(holder, input[ 0 ])
} else if (input.length === 2){
return fn(input[ 1 ], input[ 0 ])
}
return undefined
}
}

Note: just reviewed the doc, there is a small box saying Reason for the failure is... at the bottom of the flip section, but it doesn't grab the attention enough, usually a red flair or a warning icon is used in this kind of context

@selfrefactor
Copy link
Owner

I agree that the method the least should throw instead of returning undefined.

The limitation of R.flip was listed before in Differences between Rambda and Ramda section and was removed from there recently.

I thank you for pointing out that Reason for the failure is... section needs to be a bit more obvious and I will add a 💥 emoji so it reads '💥 Reason for the failure is... '.

As for R.flip - I wll try once again to make it work for 3 arguments and if I cannot make it, at least the function will throw.

@romgrk
Copy link
Contributor Author

romgrk commented May 29, 2020

Here is a version that works with 3 arguments:

function flipExport(fn) {
  const flipedFn = (...input) => {
    const missing = fn.length - input.length;

    if (missing <= 0)
      return fn(input[1], input[0], ...input)

    if (input.length === 0)
      return flipedFn

    if (input.length === 1)
      return curryN(missing, (...rest) => {
        const args = [rest[0], input[0], ...rest.slice(1)]
        return fn(...args)
      })

    // input.length >= 2
    return curryN(missing, (...rest) => {
      const args = [input[1], input[0], ...input.slice(2), ...rest]
      return fn(...args)
    })
  }

  return flipedFn
}

However it requires fn to have its .length set the to correct value, so other functions in rambda that use curry don't work (e.g. assoc) because curry drops the length :/
One easy fix could be to re-implement curry in terms of curryN:

function curry(fn) {
  return curryN(fn.length, fn)
}

@selfrefactor
Copy link
Owner

selfrefactor commented May 29, 2020

Sounds good to me. Can you open a PR for that? Just edit source/flip.js will be more than enough.

@selfrefactor
Copy link
Owner

Actually, I could make your solution work, so I increased the supported arity of Rambda.flip to 4. I could wait a few days before releasing the next version, so feel free to open a PR if you want to.

romgrk added a commit to romgrk/rambda that referenced this issue May 30, 2020
selfrefactor pushed a commit that referenced this issue May 30, 2020
* fix@issue #464 make r.flip work for any arity

* fix: lint; remove unused argument
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

Successfully merging a pull request may close this issue.

2 participants