Frank Allan Harrison
Writing Readibly
Often what you intend your code to do is not what it actually does. ... even if the code behaves exactly as is intended, what was intended may not be obvious.
That is, if the intent isn't front-and-centre, often another dev will look at your code and not immediately fully understand why it is the way it is, even if they're very talented - or even if they are you, at some point in future, with a hazy recollection of why you wrote the code. Why? Simply because they're not aware of the requirements for which the code was written. This lack of obviousness/awareness/clarity is almost always the first cause of a WTF moment. [^1]
For me, a big part of my career has been building on, and maintaining, old code. This has meant reading a lot of code, day in, day out. The problem has been that, because a good proportion of that code isn't/wasn't fully and immediately clear, it wastes a lot of my time that I could be spending in more productive ways e.g. adding features for customers, or making money for my companies.
Adjunct: you don't have to sacrifice writing code fast for it to be understood.
So, how to write code that is easier to read? Well first of all use you common sense, as there is no one-size-fits all solution, and even with a prescriptive list of rules, you need to be smart and flexible as to how and when you apply those rules.
But the question remains, how do you really write code that is easily and fully understood?
Rules of thumb
- write self documenting code.
- document why not what you intend the code to (e.g. add a "because" to the comments).
- describe the pattern or algorithm you're using.
- diagram it in comments (ascii art).
1. write self documenting code
This is probably the best and easiest thing to do. Everyone's probably heard of the first few rules here and the probably need little explanation.
1.1. meaningful variable names
Give your variables meaningful, intent-capturing, names.
For example, i might be ok in loops, simply because it's convention, but if the underlying data structure changes or you end up passing the i to a sub-function, it loses that meaning - I've seen people pop a function out of a loop but keep i as the function's parameter name, losing it's context and therefore adding more cognitive load to the reader. Instead be a tiny bit more verbose e.g. If you were using it to index into a database use personIdx or similar.
Abbreviations, like "Tbb"[^2], or "OIIO"[^3], or "PRMAN"^4, can really hurt code quality, unless they're constant across a system or industry, and documented. Even then you almost certainly shouldn't use them in your "public" APIs. Because those abbreviations might be common on your internal model of the industry, they may not mean anything to your customers - I mean who actually reads your API docs? Have you talked to anyone who has? I know that I don't want to read your docs, but I will if I have too - but I would much rather have the API be obvious and self-documented so I do not have to. If I am your customer you should help me not need to read you docs, just as I strive for you not to have to read mine.
1.1.1. dense
What does the following do? How long does it take for you to unpick the bits? How easy would it be to spot bugs?
for (int i = 0; i < cntEnt; ++i) {
Vec3 p = camMtx.prj(ents[i]->pos);
float l = calcLod(ents[i]->r, camDist(ents[i]->pos, camPos));
fsmUpd(ents[i]->st, dt);
rdrDrw(ents[i]->m, p, l, chkVis(ents[i]->f, p));
}
1.1.2. named intent
Same as above, more lines of code, higher density, but more stable and easier to reason about, long-term.
In the following can you answer the same questions? What does this do? Why does it do it? How long does it take to understand it? How easy would it be to spot bugs?
for (size_t entityIdx = 0; entityIdx < entities.size(); ++entityIdx) {
GameEntity* entityToUpdate = entities[index];
assert(entityToUpdate);
Vec3 entScreenPos = camera.worldToScreen(entityToUpdate->position);
bool entIsVisible = visibilityChecker.shouldRender(entityToUpdate->flags, screenPos);
float levelOfDetail = detailCalculator.compute(entityToUpdate->radius,
camera.distanceTo(entityToUpdate->position),
);
stateMachine.update(entityToUpdate->state, deltaTime);
renderer.drawEntity(entityToUpdate->mesh, entScreenPos, levelOfDetail, entIsVisible);
}
1.2. name your booleans
Instead of directly using complex conditions, you can capture the intent of the logic in a named bool and the code evaluates to the same assembler (as long as you use const, on most compilers).
For example which of these two is more readable? Which captures the intent more clearly and more immediately? Which has a lower cognitive load?
1.2.1. condition inside if()
// Determine if an enemy should be rendered: must be active, alive, ,
// within its LOD distance, and either not in debug-wireframe mode or flagged to show debug overlay.
if (enemy.isActive() &&
enemy.getHealth() > 0 &&
camera.frustum().contains(enemy.getBoundingBox()) &&
enemy.getPosition().distance(camera.getPosition()) <= kMaxLODDistance[enemy.getType()] &&
(!renderSettings.debugWireframeEnabled() || enemy.showDebugOverlay())) {
renderSystem.draw(enemy);
}
1.2.2. condition capture by named boolean
The logic is just as complex in the following, but the readability and intent of the if branch is much clearer. There is a separation of concerns, the control logic and the flow.
const bool enemyIsDrawable = enemy.isActive() &&
enemy.getHealth() > 0 /* alive */ &&
camera.frustum().contains(enemy.getBoundingBox()) /* inside the camera frustum */ &&
enemy.getPosition().distance(camera.getPosition()) <= kMaxLODDistance[enemy.getType()] /* LOD distance */ &&
(!renderSettings.debugWireframeEnabled() || enemy.showDebugOverlay()) /* debug render */;
if (enemyIsDrawable) {
renderSystem.draw(enemy);
}
1.3. namespaces for context
Global-namespace functions can be intractable. They can be harder to find, harder to diagnose, harder to refactor, and harder to reason about their intended usecases. But, if they are namespaced, and give a clear context where the intent is more obvious, suddenly a lot of thinking just disappears, especially when onboarding new devs, but helping everyone enter "state" by reducing cognitive load, meaning we can move faster.
One of the key considerations for namespacing is to consider how the call-sites look, how readable it is when you use the API, and NOT what the API itself looks like (as much). This is because there will be many call-sites, but just one implementation, so do you want the "many" to be readible at the (possible?) expense of the "one" or not? Is that not a reasonable expectation?
1.3.1. global fn with context encoded
How much context do you need to keep in your head reading the following code? How would you change it? What happens if you need to create a new version of the function?
// Integrates, collides, resolves & culls sleeping bodies
void physSimStepIntegrateCollideResolveAndSleepCheck(World* w, float dt);
// Call‐site
physSimStepIntegrateCollideResolveAndSleepCheck(world, deltaTime);
1.3.2. namespace and instanced scope
What is happening in this version? What needs to happen if you add a new variant of simulateFrame?
namespace Physics {
class Simulator {
public:
// Integrates, collides, resolves & culls sleeping bodies
void simulateFrame(World& world, float deltaTime);
};
}
// Call‐site
Physics::Simulator sim;
sim.simulateFrame(world, deltaTime);
2. Document why not what
Document why not what you intend the code to do ( use the word "because"). Also, I say "intend" as what the code actually does and what it was meant to do will disagree when you have bugs.
2.1 the why comment
// Pre-allocating mem for all entities up front, because we know maxEntities in advance,
// and we want the enitity data to be co-locational to reduce cache-misses.
FixedArray<Entity> entities;
entities.reserve(maxEntities);
for (FixedArray<EntityTypes>::iterator itManager = entityManagers.begin();
itManager != entityManagers.end();
++itManager)
{
// Allow for non static RAII allocations that are likely to be needed, e.g.
// up front pre-entity-type memory for co-locality.
*maxEntities.earlyReserve(maxEntities);
}
3. Capture the algorithm and/or pattern
Quite often, if is not obvious that a specific pattern is in play. Describe the pattern you're using, if you're using p-impl state it, if you're using Qt's observer pattern, state it. If you're using Template Metaprogramming, state it AND justify it. If you're using observers, adapters, thread pools, flyweight or abstract factory patterns, let the reader of the code know, in the most obvious way you can. For example, call the p-impl pointer pImpl, call the observer manager meaningfulThingObserver, call the factory object factory and so on. Such simple things really help people who are new to your code understand the intent so much faster, and if they're not used to the algorithm or pattern you've employed, because it's right there in the code, they can be curious about it and go and find more information on it.
4. diagram it in comments (ascii art)
A picture is worth a thousand words, and these days there are lot of quite nice ASCII art tools for devs.
/**
* SceneManager — orchestrator of core subsystems
*
* +-----------------+
* | SceneManager |
* +--------+--------+
* |
* +------------------+------------------+
* | | |
* +---v---+ +---v---+ +---v----+
* |Entity | |Render | |Resource|
* |Manager | |Manager| |Manager |
* +-------+ +-------+ +--------+
*
* Responsibilities:
* - EntityManager: spawn, update, destroy entities
* - RenderManager: submit, sort, draw renderables
* - ResourceManager: load, cache, unload assets
*/
class SceneManager {
public:
void initialize();
void update(float deltaTime);
void render();
void shutdown();
private:
EntityManager entityMgr;
RenderManager renderMgr;
ResourceManager resourceMgr;
};
Summary
Writing readable code saves time, and makes development much faster, especially when you're on teams that have lots of moving parts and difficult deadlines.
With practice you can elevate you and your team if you write better, easy to understand, code.
[^1]: WTFs/m – OSnews
[^2]: Thread Building Blocks (Tbb)
[^3]: Open Image IO (OIIO)