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

[NelsonMinar]

I often have this feeling when I read Mike's code, particularly D3. Then I read the code again and work to understand it and I learn something new myself. Mike's coding style leans to very terse; one letter variable names, fearless use of operator precedence and side effects. It requires you understand the language to read it clearly, but the advantage is a lot more code fits in a small space. His code is quite poetic, it's a nice change of pace from Java verbosity. (My favorite cosmetic idiom he uses: non-ASCII variable names for spherical trigonometry: https://raw.github.com/mbostock/d3/master/src/geo/conic-equi...)

[dribnet]

> I often have this feeling when I read Mike's code, particularly D3.

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));

[1] https://github.com/mbostock/d3/wiki/Selections#wiki-attr

[2] https://github.com/mbostock/d3/blob/v3.1.4/src/selection/att...

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

[ufo]

> The var statement in a while loop gives me the impression this person doesn't know how scope works in JavaScript.

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.

[xal]

It's also not a very good idea for performance. V8 optimizer will bail as soon as you do anything to arguments other then using an indexed access.

[goblin89]

IMO any overly clever code would remind of obfuscation. The trick is to notice in time your own attempts at being smart and refrain. Thus the code could communicate most of the things that needed to be explained in this walkthrough.

That's also why I personally am not very enthusiastic about such code reviews. Good code would seem boring, and why study bad code?

[pekk]

Aren't code reviews normally meant to highlight which parts are good and which are bad?

[goblin89]

Not in this case, though—OP just explains what does the code do, without any opinion.

[atomical]

So you become a better programmer?

[goblin89]

Fair enough, maybe it would work for someone. I find that reading and especially writing code of certain style shapes my style accordingly, similar to how our environment influences us, so I'd rather learn by working with good code—but it's just me.

[atomical]

Code reviews are about finding (1) the proper way to do things (2) edge cases, bugs, and the right architecture. Code reviews are not academic at all. They do require someone with a lot of experience.