How I review code
&gt; I look for code that is well-documented (both inline and externally), and code that is clear rather than clever. I’d rather read ten lines of verbose-but-understandable code than someone’s ninja-tastic one-liner that involves four nested ternaries.<p>&quot;Clear&quot; and &quot;clever&quot; aren&#x27;t in opposition, and likewise &quot;verbose&quot; and &quot;understandable&quot; aren&#x27;t correlated.<p>I think this characterisation, and especially the example, shows a lowest-common-denominator straw man of &quot;clever one-liners&quot; which seems to miss the reason that some people like them. In particular, it seems to be bikeshedding about how to write branches. The author doesn&#x27;t say what those &quot;ten lines of verbose-but-understandable code&quot; would be, but given the context I took it to mean &quot;exactly the same solution, but written with intermediate variables or if&#x2F;else blocks instead&quot;.<p>This seems like an analogous situation to <a href="https:&#x2F;&#x2F;wiki.haskell.org&#x2F;Wadler&#x27;s_Law" rel="nofollow">https:&#x2F;&#x2F;wiki.haskell.org&#x2F;Wadler&#x27;s_Law</a> where little thought is given to what the code means, more thought is given to how that meaning is encoded (e.g. ternaries vs branches) and religious crusades are dedicated to how those encodings are written down (tabs vs spaces, braces on same&#x2F;new lines, etc.).<p>Note that even in this simple example there lurks a slightly more important issue which the author could have mentioned instead: nested ternaries involve boolean expressions; every boolean expression can be rewritten in a number of ways; some of those expressions are more clear and meaningful to a human than others. For example, `loggedIn &amp;&amp; !isAdmin` seems pretty clear to me; playing around with truth tables, I found that `!(loggedIn -&gt; isAdmin)` is apparently equivalent, but it seems rather cryptic to me. This is more obvious if intermediate variables are used, since they&#x27;re easier to name if they&#x27;re meaningful.<p>In any case, compressing code by encoding the same thing with different symbols doesn&#x27;t make something &quot;clever&quot;. It&#x27;s a purely mechanical process which doesn&#x27;t involve any insights into the domain.<p>To me, code is &quot;clever&quot; if it works by exploiting some non-obvious structure&#x2F;pattern in the domain or system. For example, code which calculates a particular index&#x2F;offset in a non-obvious way, based on knowledge about invariants in the data model. Another example would be using a language construct in a way which is unusual to a human, but has the perfect semantics for the desired behaviour (e.g. duff&#x27;s device, exceptions for control flow, etc.).<p>Such &quot;clever&quot; code is <i>often</i> more terse than a &quot;straightforward&quot; alternative, but that&#x27;s a side-effect of the &quot;cleverness&quot; (finding an existing thing which behaves like the thing we want) rather than the goal.<p>If the alternative to some &quot;clever&quot; code is &quot;10 lines of verbose but understandable code&quot; then it&#x27;s probably not that clever; so it&#x27;s probably a safe bet to go with the latter. The <i>real</i> issues with clever code are:<p>- Whether the pattern it relies on is robust or subject to change. Would it end up coupling components together, or complicate implementation changes in unrelated modules?<p>- How hard it is to understand. Even if it&#x27;s non-obvious, can it be understood after a moment&#x27;s pondering; or does it require working through a textbook and several research papers?<p>- Whether the insights it relies on are enlightening or incidental, i.e. the payoff gained from figuring it out. This is more important if it&#x27;s harder to understand. Enlightening insights can change the way we understand the system&#x2F;domain, which may have many benefits going forward. Incidental insights are one-off tricks that won&#x27;t help us in the future.<p>- How difficult it would be to replace; or whether it&#x27;s possible to replace at all.<p>This last point is what annoys me in naive &quot;clever vs verbose&quot; debates, and prompted this rant, since it&#x27;s often assumed that the only difference is line count. To me, the best &quot;clever&quot; code isn&#x27;t that which reduces <i>its own</i> line count; it&#x27;s the code which <i>removes problems entirely</i>; i.e. where the alternative has caveats like &quot;before calling, make sure to...&quot;, &quot;you must manually free the resulting...&quot;, &quot;watch out for race conditions with...&quot;, etc.<p>One example which comes to mind is some Javascript I wrote to estimated prices based on user-provided sliders and tick-boxes, and some formulas and constants which sales could edit in our CMS (basically, I had to implement a spreadsheet engine).<p>Recalculating after user input was pretty gnarly, since formulas could depend on each other in arbitrary ways, resulting in infinite loops and undefined variables when I tried to do it in a &quot;straightforward&quot; way. The &quot;clever&quot; solution I came up with was to evaluate formulas and values lazily: wrapping everything in thunks and using a memo table to turn exponential calculations into linear ones. It was small, simple and heavily-commented; but the team&#x27;s unfamiliarity with concepts like lazy evaluation and memoising made it hard to get through code review.<p>Also, regarding &quot;straightforward&quot; or &quot;verbose&quot; code being &quot;readable&quot;: it&#x27;s certainly the case that any <i>particular</i> part of such code can be read and understood <i>locally</i>, but it can make the <i>overall</i> behaviour harder to understand. Just look at machine code: it&#x27;s very verbose and straightforward: &#x27;load address X into register A then add the value of register B&#x27;, simple! Yet it&#x27;s very hard to understand the &quot;big picture&quot; of what&#x27;s going on. Making code more concise, either by simplifying it or at least abstracting away low-level, nitty-gritty details into well-named functions, can help with this.<p>When used well, &quot;clever&quot; code can reframe problems into a form which have very concise solutions; not because they&#x27;ve been code-golfed, but because there&#x27;s so little left to say. This can mean the difference between a comprehensible system and a sprawling tangle of interfering patches. This may harm local reasoning in the short term, since it requires the reader to view things from that new perspective, when they may be expecting something else.<p>When used poorly, it results in things like nested ternaries, chasing conciseness without offering any deeper understanding of anything.