- Couldn't it be simpler without the linked-list queue? Use an array: the first ndispatched are for calls that have been invoked; they each hold either the corresponding result or undefined if not yet resolved by their callback. The rest of the array holds arguments objects for pending calls. When it's time to notify, this array is the results array.
- The 2-space indent plus fairly deep nesting makes it hard to see where the 'main story' starts and ends, at least for me. 4-space indent goes better with this nested style.
OTOH if the loop only ever pops at most one deferral, then an if instead of a while would make this clear.
Popping multiple synchronous tasks after an asynchronous one is fairly rare, but I’ll make sure the tests verify this code path.
It's available at http://paulirish.com/2010/10-things-i-learned-from-the-jquer... and should be interesting to people who liked this link
My own that I wrote a while back is called when-then, https://github.com/geuis/when-then
Agreed - there is tons to learn in the D3 src, but I don't feel that it is written in a way friendly to casual inspection. But D3 more than makes up for this with such clear and thorough documentation that this is rarely an issue.
Random example: wondering what a d3 selection.attr returns? Reading the docs [1] is a much quicker read than scanning the source [2] and working backwards from this nested return one-liner:
return value == null
? (name.local ? attrNullNS : attrNull) : (typeof value === "function"
? (name.local ? attrFunctionNS : attrFunction)
: (name.local ? attrConstantNS : attrConstant));
[2] https://github.com/mbostock/d3/blob/v3.1.4/src/selection/att...
> var node = arguments;
> node.i = results.push(undefined) - 1;
...
Adding properties to an arguments object, assigning the arguments object (which is not an Array, only array-like) to a variable, pushing undefined onto an array and subtracting by 1 to get the latest index.
This isn't clever, it's silly.
Simple is better than complicated. They're passing a special array-like object all over the place, note:
> slice.call(node, 1),
They do this because slice isn't on arguments. They are using an arguments outside of the function's scope.
The var statement in a while loop gives me the impression this person doesn't know how scope works in JavaScript.
The "connivence" taken with some of the other parts of the code (particularly the complicated one liners) I'll raise an eyebrow over, but they show a deep understanding of javascript. However I'm not fond of conciseness over simplicity.
I'm not the author of queue.js but I actually declaring my Javascript variables as close to their use point as possible because JSHint warnings let me pretend that JS had real block scope all along. You would be amazed how often it catches "variable used out of scope" errors that would not be caught if the declarations had been hoisted to the top of the function.
That's also why I personally am not very enthusiastic about such code reviews. Good code would seem boring, and why study bad code?