5
\$\begingroup\$

Shall I make a curry function this way, or is it best to avoid Function constructor in all means? What I want to know is if this is "bad practice" and how else currying can be done. For instance, one can bind arguments to a function with Function.bind, but it is more like a partial application thing. And will doing currying by recursion impact on performance?

var curry = function(fn) {
    var fnString = fn.toString();
    var arity = fn.length;
    var paramList = fnString.substring(
            fnString.indexOf("(") + 1, fnString.indexOf(")")).split(",");
    var body = fnString.substring(
            fnString.indexOf("return"), fnString.indexOf("}"));
    var newBody = paramList.slice(1).reduce(function(p,c) {
        return p + "return function(" + c + ") {";
    },"") + body + ( new Array(arity).join("}") );

    return new Function(paramList[0], newBody);

 };

var test = function(a,b,c,d) {
    return a * b * c * d;
};

var curriedTest = curry( test );
console.log( curriedTest(1)(2)(3)(4) ); \\-> 24
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

The problem here isn't the Function constructor in and of itself (although it's not a great idea): It's the rest of the code that'll cause trouble, I'm afraid.

You're discarding almost the entire body of the function, and retaining only the return line. And only the first return line at that.

If the test function instead looked like this:

var test = function(a,b,c,d) {
  var product = a * b * c * d;
  return product;
};

you just get an error, since the curried function becomes just:

function(a) {
  return function(b) {
    return function(c) {
      return function(d) {
        return product; // ReferenceError
      }
    }
  }
}

It'll also break if you've got nested functions, like:

var test = function(a,b,c,d) {
  function getRandomNumber() {
    return Math.random() * 10;
  }

  return a * b * c * d * getRandomNumber();
};

since your code only looks at the first return line, and thus the curried function ends up being nothing more than return Math.random() * 10.

And you can't have any close braces before a return line, so even something like this will break:

var test = function(a,b,c,d) {
  if(a == 1) {
    console.log("got: a == 1");
  }
  return a * b * c * d;
};

Since the if's } is before the return, you end up slicing the wrong stuff out of the function's body. This causes the new Function invocation to fail due to a syntax error.

Similarly, a function with no return statement will cause problems:

var test = function(a,b,c,d) {
  console.log(a * b * c * d);
};

And weird comments in the code can trip you up too:

var test = function(a,b,c,d /* more args, y'know */) { ...

Oh, and you're not propagating the context either, so within each level of nesting, this may not refer to the right/expected object.

Lastly, length is not a great way to check arity. Functions may be variadic, taking any number of arguments, whether listed or not (see below, for instance).

So, no, this is not a good way to go. Besides, the Function constructor is just eval in disguise, and should be avoided. Just use bind, or a function like below.


The usual approach is to just wrap the target function as-is. Something like:

function curry(func, args) {
  var spice = [].slice.call(arguments, 1);

  return function () {
    var args = [].slice.call(arguments);
    return func.apply(this, spice.concat(args));
  };
}

This lets you do something like:

var curried = curry(test, 1, 2, 3);
curried(4); // => 24

And it'll work for all the test function variations above.


If you do insist on rewriting code and recursively nesting things, the best I can offer (and it's not a good solution) is to not rewrite the body of the function at all.

Your current solution basically creates nested closures, meaning that the function's body can stay completely intact, only its arguments have to change. It'll just become the inner-most nested closure.

var curry = function(fn) {
  var source = fn.toString(),
      args = source.match(/\((.*?)\)/)[1].trim().split(/\s*,\s*/),
      body = source.replace(/(^.*?\{|\}[^\}]*$)/g, "");

  return args.reverse().reduce(function (nested, arg) {
    return new Function(arg, "return " + nested.toString())
  }, new Function(args.shift(), body));
};

Which produces a function like:

function anonymous(a) {
  return function anonymous(b) {
    return function anonymous(c) {
      return function anonymous(d) {
        // original function body
      }
    }
  }
}

Efficient? No. Robust? Heck no! I won't recommend using it.

But it'll work a lot better for some of the examples listed above. It'll still choke on weird comments, it doesn't do context propagation, can't handle variadics, and it's basically a big fragile hack. But at least it'll work with local variables, nested functions, additional close braces, and missing return statements.

\$\endgroup\$
2
  • \$\begingroup\$ I know there are dead cases for my curry function as it is just a draft of an idea but besides that I would rather use test.bind(null, 1,2,3)(4); \$\endgroup\$
    – django
    Commented Aug 27, 2015 at 20:08
  • \$\begingroup\$ @django The "dead cases" are everything except a one-line function...In fact, I even forgot the case of a function that only has side-effects, but no explicit return statement. That'll fail too. So you've got a ways to go from your draft. In any event, using new Function is essentially the same as using eval - there's always a better way. You're heading into voodoo territory trying to rewrite code, and I must admit I fail to see the issue in just using bind instead. \$\endgroup\$
    – Flambino
    Commented Aug 27, 2015 at 20:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.