
Analyzing mbostock's queue.js - bryansum
http://bsumm.net/2013/03/31/analyzing-mbostocks-queue-js.html
======
mbostock
This is great! Thank you for the code review. One of the aspects of Google
culture that I enjoyed the most were the asynchronous, detailed code reviews.
Knowing that someone you look up to will pore over every line of code
encourages you to get both the big picture and the details right. That happens
with less regularity in open-source, although it’s been a pleasure working
with brilliant people such as Jason Davies. So I appreciate this review
greatly; it helps not just explain the code but encourages me to do better.

~~~
abecedarius
A couple suggestions for this code:

\- 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.

~~~
mbostock
Using the results array to store the scheduled tasks is an interesting idea.
Thanks for the suggestion!

~~~
abecedarius
You're welcome! I tried coding this up at
<https://gist.github.com/darius/5287542> (untested). I see you have your own
version now, which is longer but has a loop where I fell back to the tail
call. (I also cut out a line in notify(), though you might prefer the old
behavior of passing undefined for the results array on error, vs. the new one
of an empty array. I guess I would prefer the old behavior on reflection.)

Some doc suggestions from my draft and I'll finally shut up:

    
    
        //  Can you call things after the first .await or .awaitAll?
        //  Whatever the answer, it should be documented.
        //  Maybe document the assumption that callbacks get invoked exactly once?
        //  (It'd be possible to ensure it's at most once.)
        //  Document assumption: the notified awaitAll function won't mess with its array argument
        //  (this matters if we're called again after).

------
goldfeld
That's so great, and mbostock's a coder I greatly admire. I have had it on my
mind for the past few weeks precisely this--to get into the habit of reading
code, and in doing so write deep articles explaining it. Thanks for this.

------
aqrashik
Paul Irish's "10 Things I Learned From the jQuery Source" is also a really
good analysis of the jQuery Source, particularly for people still getting
started with Javascript.

It's available at [http://paulirish.com/2010/10-things-i-learned-from-the-
jquer...](http://paulirish.com/2010/10-things-i-learned-from-the-jquery-
source/) and should be interesting to people who liked this link

~~~
mitchellhislop
The followup is also fantastic: "11 More Things I Learned From the jQuery
Source": [http://paulirish.com/2011/11-more-things-i-learned-from-
the-...](http://paulirish.com/2011/11-more-things-i-learned-from-the-jquery-
source/)

------
rlx0x
Thats a neat code review, it reminded me a little of the really awesome review
of jquery, "10 things I learned from the jquery source":
<http://vimeo.com/12529436>

~~~
Gmo
Hey thanks for that link, I learned a few nuggets in there ;)

------
geuis
Interesting, I took a deep look at his code today too over lunch. I like
seeing different people's approaches to this problem.

My own that I wrote a while back is called when-then,
<https://github.com/geuis/when-then>

------
exit
i can't decide whether this code is clever or borderline obfuscated. if i were
a more confident programmer i'd go with the latter.

~~~
btipling
I'd say I like how the thing does what it does, and I even like the overall
simplicity of each of the functions which seem easily testable. I just don't
like the JavaScript very much.

> 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.

~~~
wizard_2
Mike Bostock is the author of at least three javascript visualization
libraries that are in heavy use today. It's safe to say he's got a firm
understanding of javascript. While including a var in side a loop doesn't sope
the variables to the loop, it is convenient. You'll notice he's careful to
assign to all of the variables with each iteration and doesn't use them
outside the loop.

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.

