Skip to content
This repository

Allow return-value of tap() to replace the current value #499

Closed
wants to merge 1 commit into from

2 participants

Matt Campbell Jeremy Ashkenas
Matt Campbell

Right now it's possible to replace properties of the passed-in object to tap(), but there's no good way to replace the object itself.

This is useful if you want to use a function in your chain() that you don't want to mixin() (or the function is part of another library, etc).

Jeremy Ashkenas
Owner

That's correct. The point of tap is to return the original value -- that's all it does. If you want to use the return value of a function, just use the function.

value = func(object)
Jeremy Ashkenas jashkenas closed this March 05, 2012
Matt Campbell

@jashkenas Maybe we're not on the same page, but I thought tap was to allow you to modify the value without breaking the chain, not just return what you passed in.

In either case, value = func(object) doesn't work as you expect.

_([1, 2, 3]).chain()
.tap(function (value) {
    value = [4, 5, 6]; // this will not replace the value
})
.reverse()
.value(); // [3, 2, 1] - but expecting [6, 5, 4]
Jeremy Ashkenas
Owner

I'm really not trying to be glib, but:

[4, 5, 6].reverse()

... produces [6, 5 ,4].

Do you have a real-world example of what you're trying to write here?

Matt Campbell

Sure. I have to use some libraries at work that perform operations via copy (instead of editing inline values). Much like the string functions in JavaScript.

The above patch is just a convenient way to use non-inline operations without having to mixin() or break the chain() I am in.

Taking capitalize from the example page:

function capitalize(string) {
    return string.charAt(0).toUpperCase() + string.substring(1).toLowerCase();
}

Here is underscore now:

_('foo').chain().tap(capitalize).value(); // foo

Here is with the patch above:

_('foo').chain().tap(capitalize).value(); // Foo
Jeremy Ashkenas
Owner

Again:

_('foo').chain().tap(capitalize).value();

versus:

capitalize('foo');

... do you have any examples of real-world code where you're trying to use this?

Matt Campbell

capitalize('foo');

Not trying to be rude, but for the third time, the point of the convenience is not to break the chain of other underscore operations I'm doing (and without having to mixin() functions from other libraries)

In its current form, tap only lets you modify properties of the value - it can't change the value itself. Strings are the best real-world analogy I have for you.

I'd be happy to help you understand more but it seems like the point isn't making it across.

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

Showing 1 unique commit by 1 author.

Mar 04, 2012
Matt Campbell Allow return-value of tap() to replace the current value 6cd1568
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 7 additions and 2 deletions. Show diff stats Hide diff stats

  1. 9  underscore.js
9  underscore.js
@@ -665,8 +665,13 @@
665 665
   // The primary purpose of this method is to "tap into" a method chain, in
666 666
   // order to perform operations on intermediate results within the chain.
667 667
   _.tap = function(obj, interceptor) {
668  
-    interceptor(obj);
669  
-    return obj;
  668
+    var result = interceptor(obj);
  669
+
  670
+    if (typeof result === 'undefined') {
  671
+      return obj;
  672
+    }
  673
+
  674
+    return result;
670 675
   };
671 676
 
672 677
   // Internal recursive comparison function.
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.