Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Your comment made me look up reduce(). Just when you think you know javascript, good code like yours throws me back to researching. If I didn't see this, I would have continued to use underscorejs to do a reduce().


Hmm, with .reduce() you have:

   $('[id^=row]').toArray().reduce(function(l,r){
		return l.pipe(function(){
			return $(r).animate({opacity:opacity}, 100);
		});
	}, $.Deferred().resolve());
but without .reduce() you have:

    var pipeline = $.Deferred().resolve();
    $('[id^=row]').each(function(index, row) {
        pipeline = pipeline.pipe(function() {
            return $(row).animate({opacity:opacity}, 100);
        });
    });
(untested) which is, to my eyes at least, more readable. Just because you can do anything with .reduce() that you can do with a for loop doesn't mean you should.


The assignment in the loop breaks the readability for me:

In the reduce case, the deferred is in the scope of the function as a named argument; which is the main advantage of using deferred in this case. In the .each loop, deferred is a variable in the outer scope. Even though it looks like it will work (need to try), I wouldn't consider it the safer and more readable solution.


I guess I think of mutating a variable repeatedly as a pretty normal thing for a loop to do, nearly the only reason for having a loop, and you can see what the data flow is from the source code. In the reduce case, you kind of have to guess what the arguments to reduce() and the anonymous function are. I mean, if you'd gotten the l and r arguments backwards in your function, could you see that from looking at the code? What if you'd gotten the arguments to reduce() in the wrong order? (And you don't get to see what the initial value is until you finish reading the function that updates it.)

I think reduce() is awesome for associative operations and in general reasonable in point-free style. But I don't think its use in this case improves readability.


I agree, it is normal to mutate a variable within the loop. I find code more comfortable to maintain when the side effects are reduced. Add the pipeline variable to the .reduce:

  var pipeline = $.Deferred().resolve();
  $('[id^=row]').toArray().reduce(function(l,r){
  		return l.pipe(function(){
  			return $(r).animate({opacity:opacity}, 100);
  		});
  	}, pipeline);
But, more than avoiding mutations, thinking of the loop as a .reduce made it easier for me to write the code. The reduce has a semantic goal of computing one value out of the array, besides explicitly handling intermediate results, e.g. in this case, you can think about the .reduce as computation of a single deferred that will fire when everything completes, whereas you can get to the same effect with the .each loop by using the pipeline variable.

  var alldone = $('[id^=row]').toArray().reduce(function(l,r){
  		return l.pipe(function(){
  			return $(r).animate({opacity:opacity}, 100);
  		});
  	}, $.Deferred().resolve());
The conceptual boundary between .each and .reduce is blurred due to the two-phase nature of this code, i.e. the outer function runs as soon as the execution hits the statement and traverses all the elements, and the inner function runs as the deferred chain fires and consumes the deferreds. .reduce allowed me to think both the inner and outer functions to run as a reduction; even though the outer is really a loop.

Interesting thing about code reviews: 10% of the code generates 90% of the discussion.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: