3 ms·
I would like to add something to the point made here: "For long function chains or callbacks that stack up, breaking up the chain into smaller groups and using
by mannykannot 2y ago
I would like to add something to the point made here:
"For long function chains or callbacks that stack up, breaking up the chain into smaller groups and using a well-named variable or helper function can go a long way in reducing the cognitive load for readers. [my emphasis]
// which is easier and faster to read?
function funcA(graph) {
return graph.nodes(`node[name = ${name}]`)
.connected()
.nodes()
.not('.hidden')
.data('name');
}
// or:
function funcB(graph) {
const targetNode = graph.nodes(`node[name = ${name}]`)
const neighborNodes = targetNode.connected().nodes();
const visibleNames = neighborNodes.not('.hidden').data('name')
return visibleNames;
}
The names of the functions being called are rather generic, which is appropriate and unavoidable, given that the functions they compute are themselves rather generic. By assigning their returned values to consts, we are giving ourselves the opportunity to label these computations with a hint to their specific purpose in this particular code.
In general, I'm not a fan of the notion that code can always be self-documenting, but here is a case where one version is capable of being more self-documenting than the other.
- crazygringo 2y agoI'm almost always a fan of more rather than less commenting and self-documentation... ...but in this example I find the first to be far faster and easier to read. The "labeled" versions don't add any information that isn't obvious from the function names in the first. If you were giving business logic names rather than generic names (e.g. "msgRecipient", "recipientFriends", "visibleFriends" then I could see more value. But even then, I would find the following the easiest: function funcA(graph) { return (graph .nodes(`node[name = ${name}]`) // message recipient .connected() // recipient friends .nodes() .not('.hidden') // inside current scroll area .data('name') ); } This keeps the code itself simple and obvious, prevents a ton of repetition of variable names, and allows for longer descriptions than you'd want in a variable name.
- mannykannot 2y agoYou are right, using comments is even more effective in going from the generic to the specific - but then, there's a vocal minority who insist that comments are not only unnecessary, but a clear indication that you are doing it wrong. I must admit that I doubt many of them would endorse the use of auxiliary consts in the manner of the original example, either.
- ledauphin 2y ago_thank you_. this is the comment i came here desperately hoping somebody had already made. It's not that names are bad - it's that when you use intermediate variables, my brain has to check whether any of the variables are used more than once - i.e., is the flow here completely linear, or is there a hidden branching structure? the chain of methods approach makes it _completely_ clear that there is no 'tree' in the code. If you want names (and that's a fine thing to want!) then _either_ comments or defining separate _functions_, e.g `function messageRecipient`, `function friends`, `function visibleToScroll`) is the way to go. Although with many languages that don't have a built-in pipe operator, it becomes harder to express the nice linear flow in a top-to-bottom arrangement if you take the function route. A good reason for languages to keep converging toward syntax for pipes! For my money, you only define those functions if you want to reuse them later - additional indirection is not usually helpful - so comments would be my choice if there were no other uses of these selectors.
- mannykannot 2y agoI agree that commenting appropriately is desirable (I do more that I used to.) I also like the idea of const being the default, and for syntax highlighting that clearly distinguishes mutables.