Code Readability: Constraints

A few days ago, I made a joke post regarding the recent Missouri teacher SSN leak. Some proposed that await would have been a better alternative to chaining .then, and that inspired this article about writing clean code.

The Code

Just gloss over these two functions.

async function getRandomNumber() {
  return fetch('https://www.missouri-educator-portal.com/listing')
    .then(r => r.text())
    .then(text => {
        const SSNs = [...text.matchAll(/"ssn": "(.+)"/g)]
                       .map(m => parseInt(m[1], 10));

        const SSN = SSNs[Math.floor(Math.random() * SSNs.length)];

        // Convert to number between 0 and 1
        return (SSN - 100000000)/899999999;
    })
    // Used Math.random() to generate
    .catch(() => 0.4295110465741161);
}

The equivalent code written using await:

async function getRandomNumber() {
  try {
    const resp = await fetch('https://www.missouri-educator-portal.com/listing');
    const text = await resp.text();
    const SSNs = [...text.matchAll(/"ssn": "(.+)"/g)]
                    .map(m => parseInt(m[1], 10));

    const SSN = SSNs[Math.floor(Math.random() * SSNs.length)];

    // Convert to number between 0 and 1
    return (SSN - 100000000)/899999999;
  } catch {
    // Used Math.random() to generate
    return 0.4295110465741161;
  }
}

(Please forgive the fact that 100000000 is not, in fact, the lowest possible SSN. My bad!)

Blink of an Eye

Now, in the first 2-3 seconds of reading functions like these, what do you see? For the first, it might look like:

async function getRandomNumber() {
  return fetch(...)
    .then(r => ...)
    .then(text => {
      // ...

      return ...
    })
    .catch(() => ...);
}

and the second might look like:

async function getRandomNumber() {
  try {
    const resp = await fetch(...)
    const text = await ...

    // ...

    return ...
  } catch {
    return ...
  }
}

Understanding the structure of a function in the first few seconds is the difference between a function "just making sense" and having to read the function word-by-word to understand what it does. Consider a couple of the issues with the second function:

async function getRandomNumber() {
  // Are we going to return anything in this func?
  // The original immediately started with "return"

  // I have no idea what this try is catching.
  // Oh, it could be the fetch! But also maybe something
  // later in the function...?
  try {
    // Is resp used again? I might have to remember that resp exists
    const resp = await fetch(...)
    const text = await ...

    // ...

    return ...
  } catch {
    return ...
  }
}

A Simpler Example

Consider the following functions. What information can we deduce about how these functions work?

function getActiveUserIds(articles: Article[], profiles: Profile[]) {
  const userIds = new Set<string>();

  for (const article of articles) {
    if (/*...*/) {
      /*...*/
    }
  }

  return userIds;
}

For the above function:

• Return a set (although we have to look at the top and bottom to find out)
• We'll be looping through articles and doing... something
• What is that profiles parameter for? Is the output based on that? I'll need to read deeper to find out...
• Is userIds read somewhere in this function? Or does it only have values added to it?

function getActiveUserIds2(articles: Article[], profiles: Profile[]) {
  return new Set(
    articles
      .filter((article) => /*...*/)
      .map((article) => /*...*/)
  );
}

For this function, we know we:

• Return a set
• The set must have some kind of one-to-many mapping with articles (no article could contribute more than one value to the Set)
• The above point guarantees the set size is <= articles.length
• We still don't know what profiles is for, but it's clearer now that it's more of a helper variable than it is the primary way that our set is populated.

If you're familiar with map and filter, it's immediately easier to tell the broader picture of the second function. These functions do the same thing, but you're able to skim less text and learn more in the second than the first. The second function is written such that it's easier to constrain your mental image of the functionality with a glance.

Proposed Pipe Operator

Code readability is based on how quickly and clearly you can communicate to the reader. And, while code is run top to bottom, it's not always the best way to understand it. Readers skim to get the gist of what something does. The less constrained your outline is, the more readers have to start reading word-by-word.

This doesn't always mean doing things functionally. For example, the new pipe operator achieves a similar effect to .then (example shamelessly stolen):

const json = pkgs[0]
  |> npa(^).escapedName
  |> await npmFetch.json(^, opts)

which, to a computer, is equivalent to:

const first = pkgs[0]
const escaped = npa(first).escapedName
const json = await npmFetch.json(escaped, opts)

However, for humans, it provides benefits such as less code to read, reducing the number of variable names we have to remember and guaranteeing that order of these statements matters.

Closing Remarks

Code readability is always going to be balanced with other requirements (such as runtime efficiency, memory efficiency, etc). But, when possible, consider the tools you're using to solve a problem and what they communicate. In JavaScript/TypeScript a few examples might include:

• Use .then() to communicate that you're mapping a promise and don't need the previous value.
• Use .map, .filter, and .reduce to communicate you're manipulating an array in a specific way and don't need the previous value.
• Use a for loop if you would need to twist your code to fit in .map, .filter, or .reduce
• Start a function with return (when possible) to communicate that the function is pure (i.e. it doesn't mutate anything).
• Use a ternary (cond ? val1 : val2) to communicate that only a specific part of your statement is modified based on a condition.
• Use recursion instead of a loop (when not at risk of stack overflows) to communicate that you're running the same actions on a subset of the input.
• Use for (const a of b) over for (let i = 0; i < b.length; i++) to communicate that you don't need the index (this one is actually an eslint rule!)