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

add _ placeholders to _.partial #1276

Closed
wants to merge 3 commits into from
Closed

add _ placeholders to _.partial #1276

wants to merge 3 commits into from

Conversation

melnikov-s
Copy link
Contributor

This commit adds the ability to set palceholders for parameters when using the _.partial function. Which I feel is pretty much a required feature for a 'partial' function.

Searched previous issues before creating this pr and found: #1163. Feature was requested but no pr was ever submitted. Here's my implementation. Does not effect the speed of _.partial method which does not make use of placeholders.

var square = _.partial(_.map, _, function(n) {return n*n });
square([1,2,3,4]) //[1, 4, 9, 16]

@jashkenas
Copy link
Owner

That's very nice, but all theoretical correctness aside, a bit edge-case I think for core Underscore. Underscore-Contrib, however would be a good place for a version with placeholders. And in fact I bet if you check, it'll already be there.

@jashkenas jashkenas closed this Sep 3, 2013
@nathggns
Copy link

What? You think skipping arguments in a partial is an "edge case"?! Especially in Underscore, where arguments are almost always in the wrong order for true functional javascript?

For example:

var waitTenSeconds = _.partial(setTimeout, _, 10);

@caseywebdev
Copy link
Contributor

I didn't notice this PR originally, but I'm glad it resurfaced. I definitely agree that this is not an edge case and as @nathggns points out, makes _.partial much more useful when composing Underscore methods. As it stands, it's just hit or miss whether you can use _.partial or not based on argument order, which is frustrating. 👍

@nathggns
Copy link

I've actually written my own partial function that I almost exclusively use instead of _.partial because of its limitations.

function patial() {
    var args = _.toArray(arguments);
    var func = args.shift();

    return function() {
        var passed = _.toArray(arguments);

        var funcArgs = args.map(function(arg) {
            if (arg === null) {
                arg = passed.shift();
            }

            return arg;
        });

        funcArgs = funcArgs.concat(passed);

        if (typeof func === 'string') {
            func = this[func];
        }

        return func.apply(this, funcArgs);
    };
}

@jashkenas
Copy link
Owner

Looks like I can't reopen this for some reason -- but feel free to send another PR if you'd like.

@ryanve
Copy link
Contributor

ryanve commented Oct 19, 2013

Rather than changing _.partial, it seems simpler to implement this as a separate method called _.total. The name total applies because the total arguments count is preset, and total relates to partial. The ideal filler value is undefined. Using a loop seems easier and faster than map:

// Partially apply a function by creating a version that uses a fixed 
// number of arguments, some or all of which are pre-filled. Undefined
// items from `accept` get filled by those passed to the new function.
_.total = function(fn) {
  var accept = _.rest(arguments);
  return function() {
    for (var filled = [], i = 0, j = 0, l = accept.length; i < l; i++)
      filled[i] = void 0 === accept[i] ? arguments[j++] : accept[i];
    return fn.apply(this, filled);
  };
};

@nathggns
Copy link

I see no reason for there to be two functions that serve basically the same purpose. Why would you ever use _.partial over _.total. If you're worried about breaking existing code, that's what tests are for.

@melnikov-s
Copy link
Contributor Author

Looks like I can't reopen this for some reason -- but feel free to send another PR if you'd like.

It's because the original repo has been deleted. re-opened #1318

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 this pull request may close these issues.

None yet

5 participants