⏎ back to edhuber.net

How many times have you looked at some code and wondered, how does this work? Or even worse — what is it doing?

Chances are, you aren't the first or the last person to wonder that. That's because most code is written once, but read many times. And unfortunately, not all code is written with reviewability in mind.

Highly reviewable code empowers the reviewer to evaluate other, more interesting aspects of your code, such as correctness, or under what circumstances your code is pathological in runtime or space complexity, etc.

And for later readers of your code, highly reviewable code is also often highly debuggable (fixable). It can be easily cribbed from. It can be remixed and reused. It can even be deleted, without causing much anxiety, because the reader knows it's not secretly responsible for some other thing working.

In contrast, committed code that isn't reviewable often ends up being orphaned, misunderstood, and eventually rewritten, but not before wasting a lot of people's time, and negatively impacting the quality of the code that has to interact with it.

One class of idealized code is that code which executes the way that any passing reader would expect it to. That is, your mental model of how the code executes is what actually happens. Let's call this property obviousness. It's also been called the principle of least astonishment, but that's a mouthful.

Why focus on obviousness? Because overwhelmingly, code doesn't actually need to be efficient, concise, beautiful, or even 100% correct. It's nice if it those things too, but really the one true goal is to make reviewable code — and highly obvious code is also highly reviewable. It's not the only way to write highly reviewable code, but it's probably the easiest way.

In a nutshell, you should only make your code as complex as the problem demands. Accept that your problem is boring — because almost all problems are — and make your code unastonishing. The primary goals should be to make your intent on every line of code crystal clear, and to make it easy to follow: "connected like a string of pearls".

Here are some things I tell myself:

Don't chase "readability". Too often, readability is dependent on language-specific syntax or idioms that actually make it less obvious how something works, except for a select few.

YAGNI - you ain't gonna need it. If the code doesn't need to do X right now, then don't do X. If the code doesn't need to handle many types of things or situations yet, then don't generalize it or add interface layers.

Can I do this with simpler machinery? Does a fancy class, or lambda, or some other sophistication, really add more clarity to your code? Or am I being too clever?

Do I really need to split up this code? The prevailing dogma is that when you can, you should split up large functions into many smaller daughter functions, then call those from the parent function. I think this is correct a lot of the time, however, I also often find that the less the eye has to travel between functions or files, the easier it is to follow the intent of some code. Maybe this is one of those hard problems, like naming, where good judgment trumps any preset rules. Also see Carmack's thoughts on inlined code.

Can this method/function be clearly separated into steps? And is each step in a separate block of code, and commented?

Document and isolate irreducibly weird parts of the code. For example, suppose you find out that there's only one truly correct way to do something in Bash, and the knowledge of how that works is only in one place — say stackoverflow. You can make that a clearly separate block of code with an explanatory comment, linking to stackoverflow.

Can I put some assert()s here? Code written in a "brittle" way, such that when any of its assumptions are wrong, it will error out right there and then, inspires a lot of confidence from me. It helps explain the thought process of the author. It errors when it should, instead of potentially making a mess of things, and causing some later code to break mysteriously.

Never ignore a potential error. If something can return an error code, do something with it, even if you just assert(no error).