Forward
This document was originally published for internal use to help junior engineers collect a set of best practices for writing readable, maintainable code. Many senior engineers likely already do these things either consciously or subconsciously, but might not have identified or named the behaviors explicitly. It is also worth calling out that while this is a crowdsourced collection of opinions from several experienced Staff+ engineers, it is just that - a collection of opinions. You're welcome to disagree with any or all of those opinions!
Static Analysis
Our syntax and formatting rules are encoded with a combination of Prettier and ESLint configuration. These rules ensure both consistent readability of all code across our product (Prettier) and increase the barrier to introducing buggy or error-prone patterns (ESLint).
ESLint
Introducing New Rules
ESLint warnings tend to be ignored because they do not fail builds. All ESLint rules should be set to error, and all existing violations either fixed in the same changeset, or disabled with inline comments.
General Style
This section is aspirational about how engineers think about their code, but is not prescriptive about specific patterns. As we uncover specific patterns that match each of these buckets, add them to support the prose.
Our engineers should optimize their code for, in order:
- Correctness
- Others' ability to understand
- Author's ability to understand
This includes all aspects of submitted code, including but not limited to: variable, class, type, file, and function naming; folder hierarchies; location of functions, classes, and types within one or many files; and general readability of test suites.
Correctness
This is obviously table-stakes, and the point doesn't need to be belabored. Code that doesn't solve a problem for a customer or improve the customer experience should be questioned. Similarly, code that introduces a significant risk of breaking the customer experience (now or in the future) should be introduced sparingly.
Others' ability to understand
This is where many engineers mix up their priorities. For any unit of code, its author is not the primary audience of that code, the primary audience is all the people they work with. When introducing code, you may want to ask yourself a few questions:
- Will my code reviewer be able to understand this code quickly and easily?
- Will an on-call engineer be able to understand this code under pressure? What about at 2am when handling an urgent alert?
- Will a new engineer, contractor, or intern be able to make changes to this code safely?
- Will I be able to understand this code in 3 months? 6 months? A year?
If any of those questions aren't answerable a clear and obvious "yes," rename, reorganize, and restructure until they are. Some things to consider as solutions:
- Turn longer methods/functions into smaller, well-tested, single-purpose functions
- Rename symbols to be descriptive of their absolute task, rather than their purpose in a local context (e.g.
const recordCountis superior toconst count)- Don't be afraid of long names. Autocomplete is our friend!
- Leave explicit comments explaining "why." Trust that your fellow engineers will understand the "what"
Guiding Principles
There's a few philosophies to keep in mind that can help guide you towards code that's ultimately understandable to others:
- What story does this code tell?
- Reading a source file, class, or method should convey a clear picture of what the code is accomplishing. The same way a novel with out of order chapters or all characters having similar names would be hard to follow, code with unclear naming or structure is hard to follow. Liberal use of descriptive variable names and clearly named helper functions can convey this "story," as can comments.
- Does the written code convey its intent?
- All code has an intent. For each line or section of code, is it obvious what the code is attempting to do? This is not to say every line needs a helper function or a long descriptive variable name, but rather that the specific language constructs you use should clearly communicate the goal of each statement.
- For example,
maybeNull && arr.includes(maybeNull)conveys a different intent thanarr.includes(maybeNull ?? DEFAULT_VALUE)- one suggests that null values require different handling, the other suggests that we are strictly focused on the array's contents regardless of the element's truthiness. Picking a pattern that conveys proper intent should be prioritized over the "ideal" TypeScript way to do something.
Author's ability to understand
It is a learned skill to write code for others' consumption, and not an easy one. One thing you may notice when optimizing for your teammates' ability to understand your code is you personally will find the code easier to navigate as well. However, if there is ever a case where there is a pattern you find more understandable or simply a pattern that you personally prefer, but may not be as clear to another reader, default away from these patterns.
This is a practice that also takes some humility. Not all engineers find strictly object-oriented code easy to read. Not all engineers find strictly functional code easy to read. Knowing how to blend the two styles to maximize the "reach" of your code is something worth being mindful of, and practicing over time.
Examples of places we may tend to optimize for ourselves include, but are not limited to:
- A more-correct but highly complex TypeScript type that enforces correctness at the cost of readability (e.g. unnecessary use of mapped types when a simple
Recordwill work fine) - Terse variable names that require reading the entire surrounding context to understand
Note about PR reviews
The author of a change is unqualified to evaluate its absolute quality.
This is the core of the "humility" note above. Code review is the ultimate test of "is this code easy for others to understand?" If one of your PRs receives a surprising number of questions or comments, you may not have passed that test yet. Don't view this type of feedback as a commentary on your skill as an engineer, rather recognize this as your teammates asking for help and you're the only one in a position to modify the code to help them!
Similarly, question reviews on complex PRs with little or no feedback, as the reviewer may not have been able to grok the code well enough to review it effectively.
Testing
Test code should adhere to a similar standard for quality as product code. It's easy to make tests an afterthought because it's "just" test code, but tests are a powerful tool for both documenting intended behavior and ensuring correct behavior. Messy or difficult to read tests make it impossible to safely modify the original source for a couple of reasons:
- Confusing tests don't communicate intent
- Hard to read tests get deleted or re-written
Tests that don't communicate intent generally make it impossible to safely refactor existing classes. When someone sees a test but doesn't know what conditions it's testing, or what behavior is expected, they will likely ignore it as they refactor. If that test is failing once the refactor is complete and thought to be functioning correctly, it'll likely be rewritten to pass (potentially ignoring a real bug), or worse, deleted.
Conversely, tests that communicate a clear intended behavior can guide a refactor and make it easy to confidently rewrite a class from scratch. Implementing code to make a test pass is relatively easy, guessing at what conditions will make the test pass is hard. This is not the same as just having descriptive expectations (or just a lot of expect calls!), which tell a reader which values should be present, but not if or why they are significant.
Our tests should:
- Rely on clear (often duplicative) procedural code rather than clever or terse patterns
- Avoid abstraction and DRY in favor of simple, repeated steps (find + replace is your friend!)
- Have many suites with clear, descriptive names and a few expectations; avoid large monolithic test blocks with many expectations
Appendix
Below are some resources that can help expand your toolkit for writing clean, readable code. There's no one correct way to write "clean" code, so consider aggregating the advice you find most helpful from each resource rather than picking one style of code to follow religiously.