What makes code unreadable
Tags: readability
Published: 2018-09-28 13:11:02

---
title: What makes code unreadable
published: true
description: Any fool can write code that a computer can understand. Good programmers write code that humans can understand.
canonical_url: https://medium.com/software-engineering-problems/what-makes-code-unreadable-to-human-45fa30544386
tags: readability
cover_image: https://image.slidesharecdn.com/typography-in-web-design-wcto-2014-141115164222-conversion-gate02/95/typography-in-web-design-wordcamp-toronto-2014-41-638.jpg?cb=1416167174
---

"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](https://medium.com/@egonelbre/psychology-of-code-readability-d23b1ff1258a)
* [Why global variables are evil](https://www.learncpp.com/cpp-tutorial/4-2a-why-global-variables-are-evil/)
* [Asynchronous JavaScript: From Callback Hell to Async and Await](https://blog.hellojs.org/asynchronous-javascript-from-callback-hell-to-async-and-await-9b9ceb63c8e8)

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. 

```go
// more variable
sum1 = v.x
sum2 := sum1 + v.y
sum3 := sum2 + v.z
sum4 := sum3 + v.w
```

```go
// 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. 

```java
// longer execution path to track
public void SomeFunction(int age)
{
    if (age >= 0) {
        // Do Something
    } else {
        System.out.println("invalid age");
    }
}
```

```java
// 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

```java
// 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.

```c
// 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:

```asm
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:

```c
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

```javascript
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

```javascript
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

```java
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

```java
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

```java
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

```go
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:

```java
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

```c
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"

![image](https://user-images.githubusercontent.com/40541/46205981-27131a80-c355-11e8-8a87-c73d29df5e43.png)

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.

```go
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

```go
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.

![image](https://user-images.githubusercontent.com/40541/46206140-a99bda00-c355-11e8-9a06-30bf7a8c324b.png)

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.