PoC on new currying

[nlepage]

Issue : #297

[codecov-io]

Codecov Report

Merging #298 into dev will increase coverage by 0.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##              dev     #298      +/-   ##
==========================================
+ Coverage   99.15%   99.17%   +0.01%     
==========================================
  Files         105      106       +1     
  Lines         356      364       +8     
==========================================
+ Hits          353      361       +8     
  Misses          3        3

Impacted Files	Coverage Δ
packages/immutadot/src/core/curry.js	100% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update dfb76d4...91316ca. Read the comment docs.

[nlepage]

@frinyvonnick Ready to review

[EmrysMyrddin]

After some thinking, perhaps it is possible to have only one function curry that doesn't require the two implementations curried and maybeCurried and that doesn't rely on this.

Something like this (haven't tested it) :

export function curry(fn, minArity = fn.length) {
  const curried = previousArgs => (...args) => {
    const allArgs = previousArgs.concat(args)

    if (allArgs.length >= minArity - 1) {
      const obj = this.args[allArgs.length - 1])
      return fn(obj, ...allArgs.slice(0, -1))
    }

    return curried(allArgs)
  }

  return curried([])
}

[nlepage]

@EmrysMyrddin What you're suggesting doesn't handle a non curried call, it always takes the last parameter for obj.

[nlepage]

I like the usage of bind and this as a context, this doesn't need any dynamic function creations.

I had created the maybeCurried in order to separate concerns:

• maybeCurried chooses between curried and non curried
• curried handles curried calls

I agree that mutating the context is not a good idea at all. The first time I had written it with destructuring between each bind, I don't know why I changed it, I'm going to revert it...

[EmrysMyrddin]

For me, the bind function generate a new function when called. If it was not the case, calling bind on a function could have unexpected behavior.

[nlepage]

I read bind's documentation, it clearly states that a new function is created. They are even talking about an exotic function object, whatever this means...

I tried to make some performance tests: https://jsperf.com/binded-vs-curried/1 These doesn't seem very reliable:

• On Chrome bind is ~2% slower
• On Firefox curried is 10x faster ?!

I find Firefox's result hard to believe, I suspect it is doing some shady optimization like "hey, he doesn't use the value, let's not compute it" and it can't do the same with bind... That's why I tried to send the value to a noop function, but it doesn't change anything.

Let me know what's the result on your computers.

Anyway I'm leaning towards rewriting without using bind.

[frinyvonnick]

bind is 11% slower on Chrome for me 😄

[EmrysMyrddin]

I have the same results : bind is 4% slower on chrome and 91% slower on firefox.

[EmrysMyrddin]

Ok finally I have tried to do it with only one function. Not sure if the result is better. At least it doesn't duplicate some behavior

export function curry(fn, minArity = fn.length) {
  const curried = (previousArgs = [], last = false) => (...args) => {
    // Short circuit if no curry is necessary
    if (previousArgs.length === 0 && args.length >= minArity) return fn(...args)

    if (last) {
      const [obj] = args
      return fn(obj, ...previousArgs)
    }

    const allArgs = previousArgs.concat(args)
    const nextLast = allArgs.length >= minArity - 1

    return curried(allArgs, nextLast)
  }

  return curried()
}

[nlepage]

OK, one lined it ! Just kidding... 2 lined it.

Usually I avoid using ternary in this kind of function. What do you think ? Should I use if returns instead ?

[EmrysMyrddin]

👍 Very nice !

[frinyvonnick]

I find it less readable ! I prefer the one with if and returns 😄