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

I just wasted about 30 minutes writing a short solution, when I could have written the 22 callbacks in about 5 minutes. I did learn quite a bit about jquery, and the fn.delay isn't great. The funny thing about programming is that it does seem simple once you have a solution.

    function animate_opacity_to(opacity) {
        var elems = $('[id^=row]').get();
        (function x(){
            var el;
            if(el = elems.pop()) 
                $(el).animate({opacity:opacity},100,x);
        })();
    }


Actually, jQuery animate includes a built-in animation queueing system for this exact purpose, so callbacks really aren't necessary at all! For example:

  function animate_opacity_to(opacity) {
      $('[id^=row]').forEach(function(el) {
          $(el).animate({opacity: opacity}, {duration: 100, queue: true});
      });
  }
(haven't tested this but I think it should work.) In general, I've found that using a queue or queue-like structure can often help alleviate the dreaded "callback waterfall". Instead of saying "do this, then when you're done, do this, then when you're done, do this", you're saying "here are a list of the things I want done, do them in order and don't start the next thing in the list before the previous one is done". If you have some things in the queue that can be run simultaneously, you can combine them with a Deferred (see http://api.jquery.com/jQuery.Deferred/ )

In any case, great article - I definitely remember many similar scenarios that came up while I was learning to code. Sometimes you just gotta go with the crappy implementation you know will work rather than the efficient implementation that is beyond your current understanding :)


I tried something like your code, but it doesn't work. The queue parameter defaults to true, and it only applies to the set of elements in the jQuery object.

What will work is adding `delay` e.g.

    $(el).delay(i*100).animate(...
But that causes other problems because frantically clicking will cause the animations to get queued. Setting `queue` to false, breaks .delay().


After some reading, it seems animate attaches a deferred on the element, so this works:

  $('[id^=row]').toArray().reduce(function(l,r){
		return l.pipe(function(){
			return $(r).animate({opacity:opacity}, 100);
		});
	}, $.Deferred().resolve());
Obviously, you can put any selector in there and you'll get them animate in order. Hence, allowing you to play with many combinations, once you spend the time to write this once. I agree with the OP about getting something done is always better than nothing. Yet, the responses that s/he got from programmer friends is not an issue of technical pragmatism, but one of collaboration culture. Unfortunately colleges are like that. Things are different in more professional settings.


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.


The time difference between the dead-simple (and ugly) solution that we almost all shy away from ("it burns us!") and the more elegant solution is especially interesting.




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

Search: