_.chain & _.each are not playing together?

[LeonFedotov]

Hi I'm not sure but I think this is a bug:

  _.chain([1,2,3,4]).each(function(current){ console.log(current) }).value(); // returns undefined
  //where: 
  _.chain([1,2,3,4]).map(function(current){ return current; }).value() // returns expected. 

I guess chained function have to return values but then tap is not an iterable, im looking for a way to access the values without un-chaining them and without mutation.

[akre54]

That's correct. Unlike map, each doesn't have a return value, so calling value on it does nothing. If you need to pass a mutated array through the chain, it's best to stick with map.

[LeonFedotov]

ok so map, how about _.chain().tap(function(values) { values.forEach(function(current) {}; });
does not seem elegant but seems like its logical...

[akre54]

Yep, looks good by me:

_.chain([1,2,3,4])
 .tap(_.bind(console.log, console))
 .map(function(val) { return val * 2 })
 .value(); // [2,4,6,8]

[xixixao]

It would be nice to mention that each doesn't return a value in the documentation.

[akre54]

@xixixao each follows the semantics of Array#forEach, which also doesnt return any value. If you need something returned just use a map.

[ghost]

I don't see anything preventing Underscore from making _.each chainable. It breaks with native semantics for other methods. There are subtle differences with suggesting _.map like it doesn't return the same array and it requires a return value in the callback.

[akre54]

Why would each need to be chainable? You're not mutating the array in any way (the way you are with something like a map or filter), so therefore there is no no expectations of the value being reusable.

[ghost]

Why would each need to be chainable?

Why not allow chaining, what could it hurt? It's handy for cases like _(collection).each(intermediateActions).filter(...).

[LeonFedotov]

I would make the method undefined in a wrapped object context.

[akre54]

@Dw40 but what are you doing in those intermediate actions that can't better be handled with a map?

[ghost]

but what are you doing in those intermediate actions that can't better be handled with a map?

It doesn't really matter what the intermediate step is, it could be a number of things, the benefit of _.each chaining is that the intermediate callback doesn't have to return a value and it doesn't need to create a new array.

[akre54]

@Dw40 want to whip up a quick pull? You're talking about special-casing each as part of the mix-in right?

[xixixao]

@akre54 I know that each follows forEach, I literally meant that it would be good to mention the behavior in the documentation.

That said, I agree that each should just return the original collection for chaining (always, whatever syntax is each being called with).

[ghost]

You're talking about special-casing each as part of the mix-in right?

Simpler than that. Just have_.each return obj. No need to special case.

[akre54]

But then you're breaking compat with the spec. That's not what each does.

[ghost]

But then you're breaking compat with the spec. That's not what each does.

I previously wrote:

I don't see anything preventing Underscore from making _.each chainable. It breaks with native semantics for other methods.

I don't see that as a big deal. Underscore already parts with the spec'ed behavior for better dev usability in several methods. For example in edge _.keys(...) doesn't throw if the argument passed is not an object, and methods like _.every and _.some work if you don't pass an iterator.

[akre54]

I don't think it's a huge change, but it's definitely unexpected that each will return anything, let alone the value it was passed (and a much bigger spec break than checking for nullish arguments).

To be clear, you're talking about making this change, right?

diff --git a/underscore.js b/underscore.js
index 7a30b0a..2b56536 100644
--- a/underscore.js
+++ b/underscore.js
@@ -74,7 +74,7 @@
   // Handles objects with the built-in `forEach`, arrays, and raw objects.
   // Delegates to **ECMAScript 5**'s native `forEach` if available.
   var each = _.each = _.forEach = function(obj, iterator, context) {
-    if (obj == null) return;
+    if (obj == null) return obj;
     if (nativeForEach && obj.forEach === nativeForEach) {
       obj.forEach(iterator, context);
     } else if (obj.length === +obj.length) {
@@ -87,6 +87,7 @@
         if (iterator.call(context, obj[keys[i]], keys[i], obj) === breaker) return;
       }
     }
+    return obj;
   };

[ghost]

To be clear, you're talking about making this change, right?

Right.

[akre54]

I dont particularly care one way or another but it would be good to hear from more people about real use cases for or against this before merging.

[jashkenas]

I think you can go for it. Would be a little more useful.

[LeonFedotov]

In my opinion at least when working in a chained context, making _.each return a value is expected behavior, so +1 :)

[akre54]

@LeonFedotov @Dw40 give that a try.

[xixixao]

@akre54 Awesome.

[LeonFedotov]

@akre54 great, thanks!

[davidchambers]

Interestingly, we've reverted to the pre-1.1.3 behaviour.

The following addition to the 1.1.3 release notes was made in 3b916a2:

_.each no longer returns the iterated collection, for improved consistency with ECMA5's forEach.

[akre54]

Ha. I knew it seemed familiar. Good spot.
On Feb 1, 2014 11:23 PM, "David Chambers" notifications@github.com wrote:

Interestingly, we've reverted to the pre-1.1.3 behaviour.

The following addition to the 1.1.3 release notes was made in 3b916a23b916a2
:

_.each no longer returns the iterated collection, for improved
consistency with ECMA5's forEach.

Reply to this email directly or view it on GitHubhttps://github.com//issues/1391#issuecomment-33883825
.

[dwilches]

Is there now an alternative method that can be chained and allow modifying elements of an array in place? I was searching for something like a peek:

_.chain(myArray)
    ...
    .peek(e -> e.craftedVar = e.otherVar * 2)
    ...
    .value();