"Any fool can write code that a computer can understand. Good programmers write code that humans can understand."
Readable code is all about how our brain interpret the source code, what affects its performance. There are four major reasons that make the code unreadable:
- Too many or too long: when your brain need to track # of variables, # of lines of logic
- Non-local logic: we prefer continuous, linear and isolated logic. There are three reasons to make the logic non-local
- coding style: global variable, simd intrinsics v.s. spmd style gpu computing, callback v.s. coroutine
- generalization: to reuse the code, we need to entangle multiple execution paths into one
- non-functional requirements: it is temporal and spatial co-located (both in source code and runtime) with functional logic
- What you see is not what you get: we are forced to imagine how the code works in the runtime when it is vastly different from the source code form. Such as meta programming, multi threading with shared memory.
- Unfamiliar concept: we use name and reference to link one concept to another. When the linking is not strong, we will have trouble and say it does not "make sense"
Let's talk about them one by one. Examples taken from
- Psychology of Code Readability
- Why global variables are evil
- Asynchronous JavaScript: From Callback Hell to Async and Await
Too many or too long
We have limited power to hold variables in our working memory. Keeping track of the change in head consumes exponentially more power every step.
// more variable
sum1 = v.x
sum2 := sum1 + v.y
sum3 := sum2 + v.z
sum4 := sum3 + v.w
// less variable
sum = sum + v.x
sum = sum + v.y
sum = sum + v.z
sum = sum + v.w
Every variable has the potential to be changed. If the sum1, sum2, sum3, sum4 is const, it is less stressful.
// longer execution path to track
public void SomeFunction(int age)
{
if (age >= 0) {
// Do Something
} else {
System.out.println("invalid age");
}
}
// shorter execution path to track
public void SomeFunction(int age)
{
if (age < 0){
System.out.println("invalid age");
return;
}
// Do Something
}
Return early cuts down the execution paths we need to track in the head. The shorter path to reach a conclusion, the better.
Non-local logic
We prefer continuous, linear and isolated logic. Let me explain what I mean
- continuous: the line 2 should be related to line 1, they are put together to express close causality
- linear: you read code from top to bottom, the code executes from top to bottom
- isolated: all you need to care about is in one place
// continuous, linear, isolated
private static boolean search(int[] x, int srchint) {
for (int i = 0; i < x.length; i++)
if (srchint == x[i])
return true;
return false;
}
Logic locality is most common problem, and it is subjective. What you cares about defines what does "local" mean to you. Refactoring is re-factoring, it is all about shuffling the logic around, re-organize it in certain ways to make it readable.
There are three reasons to make the logic non-local.
- coding style: global variable, simd intrinsics v.s. spmd style gpu computing, callback v.s. coroutine
- generalization: to reuse the code, we need to entangle multiple execution paths into one
- non-functional requirements: it is temporal and spatial co-located (both in source code and runtime)
Non-local logic: coding style
Communicate through global variable will make the causality implicit, making the required logic pieces harder to be put back together.
// declare global variable
int g_mode;
void doSomething()
{
g_mode = 2; // set the global g_mode variable to 2
}
int main()
{
g_mode = 1; // note: this sets the global g_mode variable to 1. It does not declare a local g_mode variable!
doSomething();
// Programmer still expects g_mode to be 1
// But doSomething changed it to 2!
if (g_mode == 1)
std::cout << "No threat detected.\n";
else
std::cout << "Launching nuclear missiles...\n";
return 0;
}
It is trivial to translate same logic from using global variable to passing context explicitly via arguments. It is the coding style we choose to make code unreadable.
Second example is about SIMD programming. Write code to drive SIMD executor, we need to take care of multiple "data lane" at the same time. Notice the %ymm0 is 256bit register, 8 data lane for 32 bit:
LBB0_3:
vpaddd %ymm5, %ymm1, %ymm8
vblendvps %ymm7, %ymm8, %ymm1, %ymm1
vmulps %ymm0, %ymm3, %ymm7
vblendvps %ymm6, %ymm7, %ymm3, %ymm3
vpcmpeqd %ymm4, %ymm1, %ymm8
vmovaps %ymm6, %ymm7
vpandn %ymm6, %ymm8, %ymm6
vpand %ymm2, %ymm6, %ymm8
vmovmskps %ymm8, %eax
testl %eax, %eax
jne LBB0_3
Instead of specifying how to apply same operation on multiple data lane, writing code to deal with one "data lane" is much simpler:
float powi(float a, int b) {
float r = 1;
while (b--)
r *= a;
return r;
}
It takes https://ispc.github.io/ispc.html to compile the code from SPMD style to SIMD style, they are equivalent.
The third example is about callback v.s. co-routine
const verifyUser = function(username, password, callback){
dataBase.verifyUser(username, password, (error, userInfo) => {
if (error) {
callback(error)
}else{
dataBase.getRoles(username, (error, roles) => {
if (error){
callback(error)
}else {
dataBase.logAccess(username, (error) => {
if (error){
callback(error);
}else{
callback(null, userInfo, roles);
}
})
}
})
}
})
};
compared to
const verifyUser = async function(username, password){
try {
const userInfo = await dataBase.verifyUser(username, password);
const rolesInfo = await dataBase.getRoles(userInfo);
const logStatus = await dataBase.logAccess(userInfo);
return userInfo;
}catch (e){
//handle errors as needed
}
};
Co-routine makes the logic linear, going from top to bottom. The code written in callback style, going from left to right. But they are equivalent. We can choose the coding style to make code more readable, given the programming language allow us to do so.
Non-local logic: Generalization
To generalize, you have to specialize. If you need to support specialized code for 10 products sharing majority of common code. How often do you need to reason 10 products logic together? Very often, you are thinking about how 1 specific product type works. But reading the generalized code, you are forced to skip over the code for other 9 kinds. The skipping causes real cognitive load.
Here is a simple example
public double PrintBill()
{
double sum = 0;
foreach (Drink i in drinks)
{
sum += i.price;
}
drinks.Clear();
return sum;
}
We think PrintBill is too useful, we have to reuse it. But for happy hour, we need to take discount on some drink. The code become
interface BillingStrategy
{
double GetActPrice(double rawPrice);
}
// Normal billing strategy (unchanged price)
class NormalStrategy : BillingStrategy
{
public double GetActPrice(Drink drink)
{
return drink.price;
}
}
// Strategy for Happy hour (50% discount)
class HappyHourStrategy : BillingStrategy
{
public double GetActPrice(Drink drink)
{
return drink.price * 0.5;
}
}
public double PrintBill(BillingStrategy billingStrategy)
{
double sum = 0;
foreach (Drink i in drinks)
{
sum += billingStrategy.GetActPrice(i);
}
drinks.Clear();
return sum;
}
To generalize PrintBill to both happy hour and normal hour, we have to specialize on billing strategy. To read the generalized code, it is certainly less readable than original version.
Also to support all kinds of cases, the code can not be specific for just one scenario. This will create a lot of "variation point". It is very possible, for certain scenario the variation is simply a empty impl to fill the hole. For example, if we need extra service charge for normal hours. The code looks like
interface BillingStrategy
{
double GetActPrice(double rawPrice);
}
// Normal billing strategy (unchanged price)
class NormalStrategy : BillingStrategy
{
public double GetActPrice(Drink drink)
{
return drink.price;
}
public double GetExtraCharge()
{
return 1;
}
}
// Strategy for Happy hour (50% discount)
class HappyHourStrategy : BillingStrategy
{
public double GetActPrice(Drink drink)
{
return drink.price * 0.5;
}
public double GetExtraCharge()
{
return 0;
}
}
public double PrintBill(BillingStrategy billingStrategy)
{
double sum = 0;
foreach (Drink i in drinks)
{
sum += billingStrategy.GetActPrice(i);
}
sum += billingStrategy.GetExtraCharge();
drinks.Clear();
return sum;
}
If you are maintaining the logic for happy hour, the line sum += billingStrategy.GetExtraCharge(); is completely irrelevant to you. But you are forced to read it anyway.
Also there are many ways to write "branching" code. function overload, class template, object polymorphism, function object, jump table, and plain if/else. Why do we need so many ways to express a simple "if"? It is absurd.
Non-local logic: Non-functional requirements
Functional and non-functional code are tangled together making the code hard to interpret by human. The primary goal of the source code should be describing the causality chain of everything. When some x happen, but y did not follow, we perceive it as a bug, which is what I mean as causality chain. To make this causality chain executed on physical hardware, there are a lot of details to be specified. For example:
- the value is allocated on stack or heap
- the argument is passed as copy or pointer
- how to log the execution so that we can trackback when debugging
- how to execute concurrent "business" processes, on single thread, multi threads, multi machines, over multi days
Here is an example on error handling
err := json.Unmarshal(input, &gameScores)
if ShouldLog(LevelTrace) {
fmt.Println("json.Unmarshal", string(input))
}
metrics.Send("unmarshal.stats", err)
if err != nil {
fmt.Println("json.Unmarshal failed", err, string(input))
return fmt.Errorf("failed to read game scores: %v", err.Error())
}
The functional code is just json.Unmarshal(input, &gameScores), and if err != nil return. We added a lot non-functional code to handle the error. Skipping over those code is non-trivial.
What you see is not what you get
We are forced to imagine how the code works in the runtime when it is vastly different from the source code form. For example:
public class DataRace extends Thread {
private static volatile int count = 0; // shared memory
public void run() {
int x = count;
count = x + 1;
}
public static void main(String args[]) {
Thread t1 = new DataRace();
Thread t2 = new DataRace();
t1.start();
t2.start();
}
}
Count is not always x+1, given there are other thread doing the same thing in parallel, they might override your x+1 with their x+1.
Meta-programming also requires a lot of imagination power. Unlike function call, you can not jump into the function you called to checkout what you are building upon. For example
int main() {
for(int i = 0; i < 10; i++) {
char *echo = (char*)malloc(6 * sizeof(char));
sprintf(echo, "echo %d", i);
system(echo);
}
return 0;
}
Code is generated on the fly. There is no easy way to checkout the generated source code to reason about its correctness.
Unfamiliar concept
We use name and reference to link one concept to another. When the linking is not strong, we will have trouble and say it does not "make sense"
We think the game on the left is a lot easier than the game on the right. Why? Because ladder and fire are familiar concept mapped to your real life. We build new experience on top of existing experience. If the code is detached from the requirement, and detached from real life concept, it will be hard to understand.
func getThem(theList [][]int) [][]int {
var list1 [][]int
for _, x := range theList {
if x[0] == 4 {
list1 = append(list1, x)
}
}
return list1
}
is less readable than
func getFlaggedCells(gameBoard []Cell) []Cell {
var flaggedCells []Cell
for _, cell := range gameBoard {
if cell.isFlagged() {
flaggedCells = append(flaggedCells, cell)
}
}
return flaggedCells
}
type Cell []int
func (cell Cell) isFlagged() bool {
return cell[0] == 4
}
Because [][]int is unfamiliar, but []Cell mapped to life experience.
Linking concept from code to life experience is done via "name", which is just a string. Linking concept between code modules can rely on "reference". You define function, and reference the function. You define type, and reference the type. From IDE, you can click the ref to go to its definition. When the linking is strong and clear, we can read the code a lot quicker.
Concepts come from decomposition. Every time we decompose we give it a name. There are three kinds of decomposition:
- Spatial Decomposition
- Temporal Decomposition
- Layer Decomposition
When we feel the problem is big, we can always break it down by this three methods, just remember to name the concept broken down properly.
Top comments (1)
It's about tolerance.