Revise combinators to work with methods as well as functions #14

Closed
raganwald opened this Issue May 1, 2013 · 10 comments

Comments

Projects
None yet
5 participants
@raganwald

Currently, several of the combinators have, buried within them a call to .apply(null, ...). For example:

fnull: function(fun /*, defaults */) {
  var defaults = _.rest(arguments);

  return function(/*args*/) {
    var args = _.toArray(arguments);
    var sz = _.size(defaults);

    for(var i = 0; i < sz; i++) {
      if (!existy(args[i]))
        args[i] = defaults[i];
    }

    return fun.apply(null, args);
  };
}

This works perfectly for standalone functions but cannot be used on methods because it erases the current object context. If fun.apply(null, args) is replaced with fun.apply(this, args), fnull (and others with similar calls to .apply) can be used with methods as well as with standalone functions.

@fogus

This comment has been minimized.

Show comment
Hide comment
@fogus

fogus May 13, 2013

Member

My prejudices are on display here. :-) What you say seems reasonable though I'd like to see some real need before going this path. Do you have a library that uses this magic combinator mojo?

Member

fogus commented May 13, 2013

My prejudices are on display here. :-) What you say seems reasonable though I'd like to see some real need before going this path. Do you have a library that uses this magic combinator mojo?

@raganwald

This comment has been minimized.

Show comment
Hide comment
@raganwald

raganwald May 13, 2013

http://allong.es uses this technique, but it's more clearly shown in Method Combinators. Things like _.memozied and .unsplat are fantastic if you can use them with methods. Some of the others (flip, maybe partial application) less so, but it's easy to just use one consistent style throughout such at either nothing works for methods or everything works for methods.

I'm ok with whatever decision gets made, just putting the consideration on the table.

http://allong.es uses this technique, but it's more clearly shown in Method Combinators. Things like _.memozied and .unsplat are fantastic if you can use them with methods. Some of the others (flip, maybe partial application) less so, but it's easy to just use one consistent style throughout such at either nothing works for methods or everything works for methods.

I'm ok with whatever decision gets made, just putting the consideration on the table.

@raganwald

This comment has been minimized.

Show comment
Hide comment
@raganwald

raganwald May 14, 2013

Nota bene: Underscore already does this with _.partial:

// Partially apply a function by creating a version that has had some of its
// arguments pre-filled, without changing its dynamic `this` context.
_.partial = function(func) {
  var args = slice.call(arguments, 1);
  return function() {
    return func.apply(this, args.concat(slice.call(arguments)));
  };
};

Nota bene: Underscore already does this with _.partial:

// Partially apply a function by creating a version that has had some of its
// arguments pre-filled, without changing its dynamic `this` context.
_.partial = function(func) {
  var args = slice.call(arguments, 1);
  return function() {
    return func.apply(this, args.concat(slice.call(arguments)));
  };
};
@knowtheory

This comment has been minimized.

Show comment
Hide comment
@knowtheory

knowtheory May 14, 2013

Member

@raganwald i'm slightly confused and/or unconvinced.

Aren't most people referring to underscore methods via _ directly? The main underscore lib has a reference to the object as _, and end users can decide where to put the namespace w/ _.noConflict. This seems like it'd only be an issue for folks writing underscore mixins (like this lib), even in abstract.

Beyond that, where are the object contexts going to make a difference? If object methods which are mixed into underscore are all attached to the _ object, then isn't the concern a bit moot (since everyone seems to invoke underscore methods using _.methodname anyway)?

Member

knowtheory commented May 14, 2013

@raganwald i'm slightly confused and/or unconvinced.

Aren't most people referring to underscore methods via _ directly? The main underscore lib has a reference to the object as _, and end users can decide where to put the namespace w/ _.noConflict. This seems like it'd only be an issue for folks writing underscore mixins (like this lib), even in abstract.

Beyond that, where are the object contexts going to make a difference? If object methods which are mixed into underscore are all attached to the _ object, then isn't the concern a bit moot (since everyone seems to invoke underscore methods using _.methodname anyway)?

@raganwald

This comment has been minimized.

Show comment
Hide comment
@raganwald

raganwald May 14, 2013

Great question, @knowtheory. Consider this entirely bogus example:

function Item () {
  this.onHand = 0;
  this.history = [];
}

Item.prototype.ship = function (numberOfItems) {
  this.onHand -= numberOfItems;
  this.history.push(["Shipped", numberOfItems]);
  return this;
};

Item.prototype.receive = function (numberOfItems) {
  this.onHand += numberOfItems;
  this.history.push(["Received", numberOfItems]);
  return this;
};

Item.prototype.countOnHand = function (numberOfItems) {
  var adjustment = numberOfItems - this.onHand;

  if (adjustment !== 0) {
    this.history.push(["Adjusted", adjustment]);
    this.onHand = numberOfItems;
  }
  else this.history.push(["Verified", this.onHand]);
  return this;
};

Now we want a method that marks an item being out of stock:

Item.prototype.outOfStock = _.partial(Item.prototype.countOnHand, 0);

It's true that _.partial is a function executed in _ scope, but the function it returns is executed in an item's scope, and therefore respecting this makes it usable for methods as well as for free functions. This is true for almost all decorators and combinators.

Great question, @knowtheory. Consider this entirely bogus example:

function Item () {
  this.onHand = 0;
  this.history = [];
}

Item.prototype.ship = function (numberOfItems) {
  this.onHand -= numberOfItems;
  this.history.push(["Shipped", numberOfItems]);
  return this;
};

Item.prototype.receive = function (numberOfItems) {
  this.onHand += numberOfItems;
  this.history.push(["Received", numberOfItems]);
  return this;
};

Item.prototype.countOnHand = function (numberOfItems) {
  var adjustment = numberOfItems - this.onHand;

  if (adjustment !== 0) {
    this.history.push(["Adjusted", adjustment]);
    this.onHand = numberOfItems;
  }
  else this.history.push(["Verified", this.onHand]);
  return this;
};

Now we want a method that marks an item being out of stock:

Item.prototype.outOfStock = _.partial(Item.prototype.countOnHand, 0);

It's true that _.partial is a function executed in _ scope, but the function it returns is executed in an item's scope, and therefore respecting this makes it usable for methods as well as for free functions. This is true for almost all decorators and combinators.

@knowtheory

This comment has been minimized.

Show comment
Hide comment
@knowtheory

knowtheory May 14, 2013

Member

Ah right, of course. My inclination was to preserve scope and method access, but I was unsure where it mattered. Given a clearer justification, I'd double down.

Member

knowtheory commented May 14, 2013

Ah right, of course. My inclination was to preserve scope and method access, but I was unsure where it mattered. Given a clearer justification, I'd double down.

@fogus

This comment has been minimized.

Show comment
Hide comment
@fogus

fogus May 14, 2013

Member

I think that the case laid out is wholly reasonable and I will look this over right after lunch. Thanks.

Member

fogus commented May 14, 2013

I think that the case laid out is wholly reasonable and I will look this over right after lunch. Thanks.

@ritchiea

This comment has been minimized.

Show comment
Hide comment
@ritchiea

ritchiea Sep 24, 2013

I submitted a pull request with making this usable in mind.

I submitted a pull request with making this usable in mind.

@joshuacc joshuacc self-assigned this Mar 11, 2014

@joshuacc joshuacc added this to the 160.0.0 milestone Mar 12, 2014

@joshuacc

This comment has been minimized.

Show comment
Hide comment
@joshuacc

joshuacc Apr 12, 2014

Contributor

@raganwald I just pushed some commits which should allow contrib's combinators to work for methods, too. Feel free to look it over and let me know if I missed anything. Thanks!

Contributor

joshuacc commented Apr 12, 2014

@raganwald I just pushed some commits which should allow contrib's combinators to work for methods, too. Feel free to look it over and let me know if I missed anything. Thanks!

@joshuacc joshuacc closed this Apr 12, 2014

@joshuacc joshuacc modified the milestone: 0.2.3 (Underscore 1.6.0) Apr 12, 2014

@raganwald

This comment has been minimized.

Show comment
Hide comment
@raganwald

raganwald Jun 11, 2014

Sorry I missed this earlier, looks great, @joshuacc!

👍

Sorry I missed this earlier, looks great, @joshuacc!

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment