Skip to content

Instantly share code, notes, and snippets.

@mehbab

mehbab/gist:dc3a209edbf95de5a18b70303e310921

Created December 5, 2024 10:38
Show Gist options

  • Select an option

    Learn more about clone URLs
  • Save mehbab/dc3a209edbf95de5a18b70303e310921 to your computer and use it in GitHub Desktop.

Select an option

Learn more about clone URLs
Save mehbab/dc3a209edbf95de5a18b70303e310921 to your computer and use it in GitHub Desktop.
Download ZIP
Raw
gistfile1.txt
# Clean Code Notes
## Table of contents
- [Chapter 1 - Clean Code](#chapter1)
- [Chapter 2 - Meaningful Names](#chapter2)
- [Chapter 3 - Functions](#chapter3)
- [Chapter 4 - Comments](#chapter4)
- [Chapter 5 - Formatting](#chapter5)
- [Chapter 6 - Objects and Data Structures](#chapter6)
- [Chapter 7 - Error Handling](#chapter7)
- [Chapter 8 - Boundaries](#chapter8)
- [Chapter 9 - Unit Tests](#chapter9)
- [Chapter 10 - Classes](#chapter10)
- [Chapter 11 - Systems](#chapter11)
- [Chapter 12 - Emergence](#chapter12)
- [Chapter 13 - Concurrency](#chapter13)
- [Chapter 14 - Successive Refinement](#chapter14)
- [Chapter 15 - JUnit Internals](#chapter15)
- [Chapter 16 - Refactoring SerialDate](#chapter15)
- [Chapter 17 - Smells and Heuristics](#chapter17)
<a name="chapter1">
<h1>Chapter 1 - Clean Code</h1>
</a>
This Book is about good programming. It's about how to write good code, and how to transform bad code into good code.
The code represents the detail of the requirements and the details cannot be ignored or abstracted. We may create languages that are closer to the requirements. We can create tools that help us parse and assemble those requirements into formal structures. But we will never eliminate necessary precision.
### Why write bad code?
- Are you in a rush?
- Do you try to go "fast"?
- Do not you have time to do a good job?
- Are you tired of work in the same program/module?
- Does your Boss push you to finish soon?
The previous arguments could create a swamp of senseless code.
If you say "I will back to fix it later" you could fall in the [LeBlanc's law](https://en.wikipedia.org/wiki/Talk%3AList_of_eponymous_laws#Proposal_to_add_LeBlanc.27s_law) "Later equals never"
You are a professional and the code is your responsibility. Let's analyze the following anecdote:
> What if you were a doctor and had a patient who demanded that you stop all the silly hand-washing in preparation for surgery because it was taking too much time? Clearly the patient is the boss; and yet the doctor should absolutely refuse to comply. Why? Because the doctor knows more than the patient about the risks of disease and infection. It would be unprofessional (never mind criminal) for the doctor to comply with the patient.
So too it is unprofessional for programmers to bend to the will of managers who don’t understand the risks of making messes.
Maybe sometime you think in go fast to make the deadline. The only way to go fast is to keep the code as clean as possible at all times.
### What is Clean Code?
Each experienced programmer has his/her own definition of clean code, but something is clear, a clean code is a code that you can read easily. The clean code is code that has been taken care of.
In his book Uncle Bob says the next:
> Consider this book a description of the Object Mentor School of Clean Code. The techniques and teachings within are the way that we practice our art. We are willing to claim that if you follow these teachings, you will enjoy the benefits that we have enjoyed, and you will learn to write code that is clean and professional. But don’t make the mistake of thinking that we are somehow “right” in any absolute sense. There are other schools and other masters that have just as much claim to professionalism as we. It would behoove you to learn from them as well.
### The boy Scout Rule
It’s not enough to write the code well. The code has to be kept clean over time. We have all seen code rot and degrade as time passes. So we must take an active role in preventing this degradation.
It's a good practice apply the [Boy Scout Rule](http://programmer.97things.oreilly.com/wiki/index.php/The_Boy_Scout_Rule)
> Always leave the campground cleaner than you found it.
<a name="chapter2">
<h1>Chapter 2 - Meaningful Names</h1>
</a>
Names are everywhere in software. Files, directories, variables functions, etc. Because we do so much of it. We have better do it well.
### Use Intention-Revealing Names
It is easy to say that names reveal intent. Choosing good names takes time, but saves more than it takes. So take care with your names and change them when you find better ones.
The name of a variable, function or class, should answer all the big questions. It should tell you why it exists, what it does, and how is used. **If a name requires a comment, then the name does not reveals its intent**.
| Does not reveals intention | Reveals intention |
| -------------------------------- | ----------------------- |
| `int d; // elapsed time in days` | `int elapsedTimeInDays` |
Choosing names that reveal intent can make much easier to understand and change code. Example:
```java
public List<int[]> getThem() {
List<int[]> list1 = new ArrayList<int[]>();
for (int[] x : theList)
if (x[0] == 4)
list1.add(x);
return list1;
}
```
This code is simple, but create many questions:
1. What is the content of `theList`?
2. What is the significance of the item `x[0]` in the list?.
3. Why we compare `x[0]` vs `4`?
4. How would i use the returned list?
The answers to these questions are not present in the code sample, but they could have been. Say that we’re working in a mine sweeper game. We can refactor the previous code as follows:
```java
public List<int[]> getFlaggedCells() {
List<int[]> flaggedCells = new ArrayList<int[]>();
for (int[] cell : gameBoard)
if (cell[STATUS_VALUE] == FLAGGED)
flaggedCells.add(cell);
return flaggedCells;
}
```
Now we know the next information:
1. `theList` represents the `gameBoard`
2. `x[0]` represents a cell in the board and `4` represents a flagged cell
3. The returned list represents the `flaggedCells`
Notice that the simplicity of the code has not changed. It still has exactly the same number of operators and constants, with exactly the same number of nesting levels. But the code has become much more explicit.
We can improve the code writing a simple class for cells instead of using an array of `ints`. It can include an **intention-revealing function** (called it `isFlagged`) to hide the magic numbers. It results in a new function of the function.
```java
public List<Cell> getFlaggedCells() {
List<Cell> flaggedCells = new ArrayList<Cell>();
for (Cell cell : gameBoard)
if (cell.isFlagged())
flaggedCells.add(cell);
return flaggedCells;
}
```
### Avoid Disinformation
Programmers must avoid leaving false clues that obscure the meaning of code. We should avoid words whose entrenched meaning vary from our intended meaning.
Do not refer to a grouping of accounts as an `accountList` unless it's actually a `List`. The word `List` means something specific to programmers. If the container holding the accounts is not actually a List, it may lead to false conclusions. So `accountGroup` or `bunchOfAccounts` or just plain `accounts` would be better.
Beware of using names which vary in small ways. How long does it take to spot the subtle difference between a `XYZControllerForEfficientHandlingOfStrings` in one module and, somewhere a little more distant, `XYZControllerForEfficientStorageOfStrings`? The words have frightfully similar shapes
### Make Meaningful Distinctions
Programmers create problems for themselves when they write code solely to satisfy a compiler or interpreter. For example because you can't use the same name to refer two different things in the same scope, you might be tempted to change one name in an arbitrary way. Sometimes this is done by misspelling one, leading to the surprising situation where correcting spelling errors leads to an inability to compile. Example, you create the variable `klass`because the name `class` was used for something else.
In the next function, the arguments are noninformative, `a1` and `a2` doesn't provide clues to the author intention.
```java
public static void copyChars(char a1[], char a2[]) {
for (int i = 0; i < a1.length; i++) {
a2[i] = a1[i];
}
}
```
We can improve the code selecting more explicit argument names:
```java
public static void copyChars(char source[], char destination[]) {
for (int i = 0; i < source.length; i++) {
destination[i] = source[i];
}
}
```
Noise words are another meaningless distinction. Imagine that you have a Product class. If you have another called `ProductInfo` or `ProductData`, you have made the names different without making them mean anything different. Info and Data are indistinct noise words like a, an, and the.
Noise words are redundant. The word variable should never appear in a variable name. The word table should never appear in a table name.
### Use Pronounceable Names
Imagine you have the variable `genymdhms` (Generation date, year, month, day, hour, minute and second) and imagine a conversation where you need talk about this variable calling it "gen why emm dee aich emm ess". You can consider convert a class like this:
```java
class DtaRcrd102 {
private Date genymdhms;
private Date modymdhms;
private final String pszqint = "102";
/* ... */
};
```
To
```java
class Customer {
private Date generationTimestamp;
private Date modificationTimestamp;;
private final String recordId = "102";
/* ... */
};
```
### Use Searchable Names
Single-letter names and numeric constants have a particular problem in that they are not easy to locate across a body of text.
### Avoid Encoding
We have enough encodings to deal with without adding more to our burden. Encoding type or scope information into names simply adds an extra burden of deciphering. Encoded names are seldom pronounceable and are easy to mis-type. An example of this, is the use of the [Hungarian Notation](https://en.wikipedia.org/wiki/Hungarian_notation) or the use of member prefixes.
#### Interfaces and Implementations
These are sometimes a special case for encodings. For example, say you are building an ABSTRACT FACTORY for the creation of shapes. This factory will be an interface and will be implemented by a concrete class. What should you name them? `IShapeFactory` and `ShapeFactory`? Is preferable to leave interfaces unadorned.I don’t want my users knowing that I’m handing them an interface. I just want them to know that it’s a `ShapeFactory`. So if I must encode either the interface or the implementation, I choose the implementation. Calling it `ShapeFactoryImp`, or even the hideous `CShapeFactory`, is preferable to encoding the interface.
### Avoid Mental Mapping
Readers shouldn't have to mentally translate your names into other names they already know.
One difference between a smart programmer and a professional programmer is that the professional understands that clarity is king. Professionals use their powers for good and write code that others can understand.
### Class Names
Classes and objects should have noun or noun phrase names like `Customer`, `WikiPage`, `Account`, and `AddressParser`. Avoid words like `Manager`,`Processor`, `Data`, or `Info` in the name of a class. A class name should not be a verb.
### Method Names
Methods should have verb or verb phrase names like `postPayment`, `deletePage` or `save`. Accessors, mutators, and predicates should be named for their value and prefixed with get, set, and is according to the javabean standard.
When constructors are overloaded, use static factory methods with names that describe the arguments. For example:
```java
Complex fulcrumPoint = Complex.FromRealNumber(23.0);
```
Is generally better than
```java
Complex fulcrumPoint = new Complex(23.0);
```
Consider enforcing their use by making the corresponding constructors private.
### Don't Be Cute
| Cute name | Clean name |
| ----------------- | ------------- |
| `holyHandGranade` | `deleteItems` |
| `whack` | `kill` |
| `eatMyShorts` | `abort` |
### Pick one word per concept
Pick one word for one abstract concept and stick with it. For instance, it’s confusing to have fetch, retrieve, and get as equivalent methods of different classes.
### Don’t Pun
Avoid using the same word for two purposes. Using the same term for two different ideas is essentially a pun.
Example: in a class use `add` for create a new value by adding or concatenating two existing values and in another class use `add` for put a simple parameter in a collection, it's a better options use a name like `insert` or `append` instead.
### Use Solution Domain Names
Remember that the people who read your code will be programmers. So go ahead and use computer science (CS) terms, algorithm names, pattern names, math terms, and so forth.
### Use Problem Domain Names
When there is no “programmer-eese” for what you’re doing, use the name from the problem domain. At least the programmer who maintains your code can ask a domain expert what it means.
### Add Meaningful context
There are a few names which are meaningful in and of themselves—most are not. Instead, you need to place names in context for your reader by enclosing them in well-named classes, functions, or namespaces. When all else fails, then prefixing the name may be necessary as a last resort
Variables like: `firstName`, `lastName`, `street`, `city`, `state`. Taken together it's pretty clear that they form an address, but, what if you saw the variable state being used alone in a method?, you could add context using prefixes like: `addrState` at least readers will understand that the variable is part of a large structure. Of course, a better solution is to create a class named `Address` then even the compiler knows that the variables belong to a bigger concept
### Don’t Add Gratuitous Context
In an imaginary application called “Gas Station Deluxe,” it is a bad idea to prefix every class with GSD. Example: `GSDAccountAddress`
Shorter names are generally better than longer ones, so long as they are clear. Add no more context to a name than is necessary.
<a name="chapter3">
<h1>Chapter 3 - Functions</h1>
</a>
Functions are the first line of organization in any topic.
### Small!!
The first rule of functions is that they should be small. The second rule of functions is that they should be smaller than that.
#### Blocks and Indenting
This implies that the blocks within `if` statements, `else` statements, `while` statements, and so on should be one line long. Probably that line should be a function call. Not only does this keep the enclosing function small, but also adds documentary value because the function called within the block can have a nicely descriptive name.
This also implies that functions should not be large enough to hold nested structures. Therefore, the indent level of a function should not be greater than one or two. This, of course, makes the functions easy to read and understand.
### Do One Thing
**FUNCTIONS SHOULD DO ONE THING. THEY SHOULD DO IT WELL. THEY SHOULD DO IT ONLY.**
#### Sections within Functions
If you have a function divided in sections like _declarations_, _initialization_ etc, it's a obvious symptom of the function is doing more than one thing. Functions that do one thing cannot be reasonably divided into sections.
### One Level of Abstraction per Function
In order to make sure our functions are doing "one thing", we need to make sure that the statements within our function are all at the same level of abstraction.
#### Reading Code from Top to Bottom: _The Stepdown Rule_
We want the code to read like a top-down narrative. 5 We want every function to be followed by those at the next level of abstraction so that we can read the program, descending one level of abstraction at a time as we read down the list of functions.
To say this differently, we want to be able to read the program as though it were a set
of TO paragraphs, each of which is describing the current level of abstraction and referencing subsequent TO paragraphs at the next level down.
```
- To include the setups and teardowns, we include setups, then we include the test page content, and then we include the teardowns.
- To include the setups, we include the suite setup if this is a suite, then we include the regular setup.
- To include the suite setup, we search the parent hierarchy for the “SuiteSetUp” page and add an include statement with the path of that page.
- To search the parent...
```
It turns out to be very difficult for programmers to learn to follow this rule and write functions that stay at a single level of abstraction. But learning this trick is also very important. It is the key to keeping functions short and making sure they do “one thing.” Making the code read like a top-down set of TO paragraphs is an effective technique for keeping the abstraction level consistent.
### Switch Statements
It’s hard to make a small switch statement. 6 Even a switch statement with only two cases is larger than I’d like a single block or function to be. It’s also hard to make a switch statement that does one thing. By their nature, switch statements always do N things. Unfortunately we can’t always avoid switch statements, but we can make sure that each switch statement is buried in a low-level class and is never repeated. We do this, of course, with polymorphism.
### Use Descriptive Names
> You know you are working on clean code when each routine turns out to be pretty much what you expected
Half the battle to achieving that principle is choosing good names for small functions that do one thing. The smaller and more focused a function is, the easier it is to choose a descriptive name.
Don’t be afraid to make a name long. A long descriptive name is better than a short enigmatic name. A long descriptive name is better than a long descriptive comment. Use a naming convention that allows multiple words to be easily read in the function names, and then make use of those multiple words to give the function a name that says what it does.
Choosing descriptive names will clarify the design of the module in your mind and help you to improve it. It is not at all uncommon that hunting for a good name results in a favorable restructuring of the code.
### Function arguments
The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification—and then shouldn’t be used anyway.
Arguments are even harder from a testing point of view. Imagine the difficulty of writing all the test cases to ensure that all the various combinations of arguments work properly. If there are no arguments, this is trivial. If there’s one argument, it’s not too hard. With two arguments the problem gets a bit more challenging. With more than two arguments, testing every combination of appropriate values can be daunting.
Output arguments are harder to understand than input arguments. When we read a function, we are used to the idea of information going in to the function through arguments and out through the return value. We don’t usually expect information to be going out through the arguments. So output arguments often cause us to do a double-take.
#### Common Monadic Forms
There are two very common reasons to pass a single argument into a function. You may be asking a question about that argument, as in `boolean fileExists(“MyFile”)` . Or you may be operating on that argument, transforming it into something else and returning it. For example, `InputStream fileOpen(“MyFile”)` transforms a file name `String` into an `InputStream` return value. These two uses are what readers expect when they see a function. You should choose names that make the distinction clear, and always use the two forms in a consistent context.
#### Flag Arguments
Flag arguments are ugly. Passing a boolean into a function is a truly terrible practice. It immediately complicates the signature of the method, loudly proclaiming that this function does more than one thing. It does one thing if the flag is `true` and another if the flag is `false`!
#### Dyadic Functions
A function with two arguments is harder to understand than a monadic function. For example, `writeField(name)` is easier to understand than `writeField(output-Stream, name)`
There are times, of course, where two arguments are appropriate. For example, `Point p = new Point(0,0);` is perfectly reasonable. Cartesian points naturally take two arguments.
Even obvious dyadic functions like assertEquals(expected, actual) are problematic. How many times have you put the actual where the expected should be? The two arguments have no natural ordering. The expected, actual ordering is a convention that requires practice to learn.
Dyads aren’t evil, and you will certainly have to write them. However, you should be aware that they come at a cost and should take advantage of what mechanims may be available to you to convert them into monads. For example, you might make the writeField method a member of outputStream so that you can say outputStream. writeField(name) . Or you might make the outputStream a member variable of the current class so that you don’t have to pass it. Or you might extract a new class like FieldWriter that takes the outputStream in its constructor and has a write method.
#### Triads
Functions that take three arguments are significantly harder to understand than dyads. The issues of ordering, pausing, and ignoring are more than doubled. I suggest you think very carefully before creating a triad.
#### Argument Objects
Compare:
```java
Circle makeCircle(double x, double y, double radius);
```
vs
```java
Circle makeCircle(Point center, double radius);
```
#### Verbs and Keywords
Choosing good names for a function can go a long way toward explaining the intent of the function and the order and intent of the arguments. In the case of a monad, the function and argument should form a very nice verb/noun pair. For example, `write(name)` is very evocative. Whatever this “name” thing is, it is being “written.” An even better name might be `writeField(name)` , which tells us that the "name" thing is a "field".
This last is an example of the keyword form of a function name. Using this form we encode the names of the arguments into the function name. For example, `assertEquals` might be better written as `assertExpectedEqualsActual(expected, actual)`. This strongly mitigates the problem of having to remember the ordering of the arguments.
### Output Arguments
In general output arguments should be avoided. If your function must change the state of something, have it change the state of its owning object.
### Command Query Separation
Functions should either do something or answer something, but not both. Either your function should change the state of an object, or it should return some information about that object. Doing both often leads to confusion.
### Prefer Exceptions to Returning Error Codes
Returning error codes from command functions is a subtle violation of command query separation.
### Don't Repeat Yourself
Duplication may be the root of all evil in software. Many principles and practices have been created for the purpose of controlling or eliminating it.
### Structured Programming
Some programmers follow Edsger Dijkstra’s rules of structured programming. Dijkstra said that every function, and every block within a function, should have one entry and one exit. Following these rules means that there should only be one return statement in a function, no `break` or `continue` statements in a loop, and never, _ever_, any `goto` statements.
While we are sympathetic to the goals and disciplines of structured programming, those rules serve little benefit when functions are very small. It is only in larger functions that such rules provide significant benefit.
So if you keep your functions small, then the occasional multiple `return` , `break` , or `continue` statement does no harm and can sometimes even be more expressive than the single-entry, single-exit rule. On the other hand, `goto` only makes sense in large functions, so it should be avoided
<a name="chapter4">
<h1>Chapter 4 - Comments</h1>
</a>
Nothing can be quite so helpful as a well-placed comment. Nothing can clutter up a module more than frivolous dogmatic comments. Nothing can be quite so damaging as an old comment that propagates lies and misinformation.
If our programming languages were expressive enough, or if we had the talent to subtly wield those languages to express our intent, we would not need comments very much—perhaps not at all.
### Comments Do Not Make Up for Bad Code
Clear and expressive code with few comments is far superior to cluttered and complex code with lots of comments. Rather than spend your time writing the comments that explain the mess you’ve made, spend it cleaning that mess.
### Explain Yourself in Code
```java
// Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) && (employee.age > 65))
```
vs
```java
if (employee.isEligibleForFullBenefits())
```
### Good Comments
Some comments are necessary or beneficial. However the only truly good comment is the comment you found a way not to write.
#### Legal Comments
Sometimes our corporate coding standards force us to write certain comments for legal reasons. For example, copyright and authorship statements are necessary and reasonable things to put into a comment at the start of each source file.
#### Informative Comments
It is sometimes useful to provide basic information with a comment. For example, consider this comment that explains the return value of an abstract method:
```java
// Returns an instance of the Responder being tested.
protected abstract Responder responderInstance();
```
A comment like this can sometimes be useful, but it is better to use the name of the function to convey the information where possible. For example, in this case the comment could be made redundant by renaming the function: `responderBeingTested`.
#### Explanation of Intent
Sometimes a comment goes beyond just useful information about the implementation and provides the intent behind a decision. Example:
```java
public int compareTo(Object o)
{
if(o instanceof WikiPagePath)
{
WikiPagePath p = (WikiPagePath) o;
String compressedName = StringUtil.join(names, "");
String compressedArgumentName = StringUtil.join(p.names, "");
return compressedName.compareTo(compressedArgumentName);
}
return 1; // we are greater because we are the right type.
}
```
#### Clarification
Sometimes it is just helpful to translate the meaning of some obscure argument or return value into something that's readable. In general it is better to find a way to make that argument or return value clear in its own right; but when its part of the standard library, or in code that you cannot alter, then a helpful clarifying comment can be useful.
#### Warning of concequences
Sometimes it is useful to warn other programmers about certain consequences.
```java
// Don't run unless you
// have some time to kill.
public void _testWithReallyBigFile() {
writeLinesToFile(10000000);
response.setBody(testFile);
response.readyToSend(this);
String responseString = output.toString();
assertSubString("Content-Length: 1000000000", responseString);
assertTrue(bytesSent > 1000000000);
}
```
#### TODO Comments
It is sometimes reasonable to leave “To do” notes in the form of //TODO comments. In the
following case, the TODO comment explains why the function has a degenerate implementation and what that function's future should be.
```java
//TODO-MdM these are not needed
// We expect this to go away when we do the checkout model
protected VersionInfo makeVersion() throws Exception {
return null;
}
```
TODOs are jobs that the programmer thinks should be done, but for some reason can’t do at the moment. It might be a reminder to delete a deprecated feature or a plea for someone else to look at a problem. It might be a request for someone else to think of a better name or a reminder to make a change that is dependent on a planned event. Whatever else a TODO might be, it is not an excuse to leave bad code in the system.
#### Amplification
A comment may be used to amplify the importance of something that may otherwise seem inconsequential.
```java
String listItemContent = match.group(3).trim();
// the trim is real important. It removes the starting
// spaces that could cause the item to be recognized
// as another list.
new ListItemWidget(this, listItemContent, this.level + 1);
return buildList(text.substring(match.end()));
```
#### Javadocs in Public APIs
There is nothing quite so helpful and satisfying as a well-described public API. The javadocs for the standard Java library are a case in point. It would be difficult, at best, to write Java programs without them.
### Bad Comments
Most comments fall into this category. Usually they are crutches or excuses for poor code or justifications for insufficient decisions, amounting to little more than the programmer talking to himself.
#### Mumbling
Plopping in a comment just because you feel you should or because the process requires it, is a hack. If you decide to write a comment, then spend the time necessary to make sure it is the best comment you can write. Example:
```java
public void loadProperties() {
try {
String propertiesPath = propertiesLocation + "/" + PROPERTIES_FILE;
FileInputStream propertiesStream = new FileInputStream(propertiesPath);
loadedProperties.load(propertiesStream);
}
catch(IOException e) {
// No properties files means all defaults are loaded
}
}
```
What does that comment in the catch block mean? Clearly meant something to the author, but the meaning not come though all that well. Apparently, if we get an `IOException`, it means that there was no properties file; and in that case all the defaults are loaded. But who loads all the defaults?
#### Redundant Comments
```java
// Utility method that returns when this.closed is true. Throws an exception
// if the timeout is reached.
public synchronized void waitForClose(final long timeoutMillis) throws Exception {
if(!closed) {
wait(timeoutMillis);
if(!closed)
throw new Exception("MockResponseSender could not be closed");
}
}
```
What purpose does this comment serve? It’s certainly not more informative than the code. It does not justify the code, or provide intent or rationale. It is not easier to read than the code. Indeed, it is less precise than the code and entices the reader to accept that lack of precision in lieu of true understanding.
#### Misleading comments
Sometimes, with all the best intentions, a programmer makes a statement in his comments that isn't precise enough to be accurate. Consider for another moment the example of the previous section. The method does not return when `this.closed` becomes `true`. It returns if `this.closed` is `true`; otherwise, it waits for a blind time-out and then throws an exception if `this.closed` is still not true.
#### Mandated Comments
It is just plain silly to have a rule that says that every function must have a javadoc, or every variable must have a comment. Comments like this just clutter up the code, propagate lies, and lend to general confusion and disorganization.
```java
/**
*
* @param title The title of the CD
* @param author The author of the CD
* @param tracks The number of tracks on the CD
* @param durationInMinutes The duration of the CD in minutes
*/
public void addCD(String title, String author, int tracks, int durationInMinutes) {
CD cd = new CD();
cd.title = title;
cd.author = author;
cd.tracks = tracks;
cd.duration = duration;
cdList.add(cd);
}
```
#### Journal Comments
Sometimes people add a comment to the start of a module every time they edit it. Example:
```java
* Changes (from 11-Oct-2001)
* --------------------------
* 11-Oct-2001 : Re-organised the class and moved it to new package com.jrefinery.date (DG);
* 05-Nov-2001 : Added a getDescription() method, and eliminated NotableDate class (DG);
* 12-Nov-2001 : IBD requires setDescription() method, now that NotableDate class is gone (DG); Changed getPreviousDayOfWeek(),
getFollowingDayOfWeek() and getNearestDayOfWeek() to correct bugs (DG);
* 05-Dec-2001 : Fixed bug in SpreadsheetDate class (DG);
```
Today we have source code control systems, we don't need this type of logs.
#### Noise Comments
The comments in the follow examples doesn't provides new information.
```java
/**
* Default constructor.
*/
protected AnnualDateRule() {
}
```
```java
/** The day of the month. */
private int dayOfMonth;
```
Javadocs comments could enter in this category. Many times they are just redundant noisy comments written out of some misplaced desire to provide documentation.
#### Don’t Use a Comment When You Can Use a Function or a Variable
Example:
```java
// does the module from the global list <mod> depend on the
// subsystem we are part of?
if (smodule.getDependSubsystems().contains(subSysMod.getSubSystem()))
```
vs
```java
ArrayList moduleDependees = smodule.getDependSubsystems();
String ourSubSystem = subSysMod.getSubSystem();
if (moduleDependees.contains(ourSubSystem))
```
#### Position Markers
This type of comments are noising
```java
// Actions //////////////////////////////////
```
#### Closing Brace Comments
Example:
```java
public class wc {
public static void main(String[] args) {
BufferedReader in = new BufferedReader(new InputStreamReader(System.in));
String line;
int lineCount = 0;
int charCount = 0;
int wordCount = 0;
try {
while ((line = in.readLine()) != null) {
lineCount++;
charCount += line.length();
String words[] = line.split("\\W");
wordCount += words.length;
} //while
System.out.println("wordCount = " + wordCount);
System.out.println("lineCount = " + lineCount);
System.out.println("charCount = " + charCount);
} // try
catch (IOException e) {
System.err.println("Error:" + e.getMessage());
} //catch
} //main
```
You could break the code in small functions instead to use this type of comments.
#### Attributions and Bylines
Example:
`/* Added by Rick */`
The VCS can manage this information instead.
#### Commented-Out Code
```java
InputStreamResponse response = new InputStreamResponse();
response.setBody(formatter.getResultStream(), formatter.getByteCount());
// InputStream resultsStream = formatter.getResultStream();
// StreamReader reader = new StreamReader(resultsStream);
// response.setContent(reader.read(formatter.getByteCount()));
```
If you don't need anymore, please delete it, you can back later with your VCS if you need it again.
#### HTML Comments
HTML in source code comments is an abomination, as you can tell by reading the code below.
```java
/**
* Task to run fit tests.
* This task runs fitnesse tests and publishes the results.
* <p/>
* <pre>
* Usage:
* &lt;taskdef name=&quot;execute-fitnesse-tests&quot;
* classname=&quot;fitnesse.ant.ExecuteFitnesseTestsTask&quot;
* classpathref=&quot;classpath&quot; /&gt;
* OR
* &lt;taskdef classpathref=&quot;classpath&quot;
* resource=&quot;tasks.properties&quot; /&gt;
* <p/>
* &lt;execute-fitnesse-tests
* suitepage=&quot;FitNesse.SuiteAcceptanceTests&quot;
* fitnesseport=&quot;8082&quot;
* resultsdir=&quot;${results.dir}&quot;
* resultshtmlpage=&quot;fit-results.html&quot;
* classpathref=&quot;classpath&quot; /&gt;
* </pre>
*/
```
#### Nonlocal Information
If you must write a comment, then make sure it describes the code it appears near. Don't offer systemwide information in the context of a local comment.
#### Too Much Information
Don't put interesting historical discussions or irrelevant descriptions of details into your comments.
#### Inobvious Connection
The connection between a comment and the code it describes should be obvious. If you are going to the trouble to write a comment, then at least you'd like the reader to be able to look at the comment and the code and understand what the comment is talking about
#### Function Headers
Short functions don’t need much description. A well-chosen name for a small function that does one thing is usually better than a comment header.
#### Javadocs in Nonpublic Code
Javadocs are for public APIs, in nonpublic code could be a distraction more than a help.
<a name="chapter5">
<h1>Chapter 5 - Formatting</h1>
</a>
Code formatting is important. It is too important to ignore and it is too important to treat religiously. Code formatting is about communication, and communication is the professional developer’s first order of business.
### Vertical Formatting
#### Vertical Openness Between Concepts
This concept consist in how to you separate concepts in your code, In the next example we can appreciate it.
```java
package fitnesse.wikitext.widgets;
import java.util.regex.*;
public class BoldWidget extends ParentWidget {
public static final String REGEXP = "'''.+?'''";
private static final Pattern pattern = Pattern.compile("'''(.+?)'''",
Pattern.MULTILINE + Pattern.DOTALL
);
public BoldWidget(ParentWidget parent, String text) throws Exception {
super(parent);
Matcher match = pattern.matcher(text);
match.find();
addChildWidgets(match.group(1));
}
public String render() throws Exception {
StringBuffer html = new StringBuffer("<b>");
html.append(childHtml()).append("</b>");
return html.toString();
}
}
```
```java
package fitnesse.wikitext.widgets;
import java.util.regex.*;
public class BoldWidget extends ParentWidget {
public static final String REGEXP = "'''.+?'''";
private static final Pattern pattern = Pattern.compile("'''(.+?)'''",
Pattern.MULTILINE + Pattern.DOTALL);
public BoldWidget(ParentWidget parent, String text) throws Exception {
super(parent);
Matcher match = pattern.matcher(text); match.find(); addChildWidgets(match.group(1));
}
public String render() throws Exception { StringBuffer html = new StringBuffer("<b>"); html.append(childHtml()).append("</b>"); return html.toString();
}
}
```
As you can see, the readability of the first example is greater than that of the second.
#### Vertical Density
The vertical density implies close association. So lines of code that are tightly related should appear vertically dense. Check the follow example:
```Java
public class ReporterConfig {
/**
* The class name of the reporter listener */
private String m_className;
/**
* The properties of the reporter listener */
private List<Property> m_properties = new ArrayList<Property>();
public void addProperty(Property property) { m_properties.add(property);
}
```
```java
public class ReporterConfig {
private String m_className;
private List<Property> m_properties = new ArrayList<Property>();
public void addProperty(Property property) {
m_properties.add(property);
}
}
```
The second code it's much easier to read. It fits in an "eye-full".
#### Vertical Distance
**Variable Declarations**. Variables should be declared as close to their usage as possible. Because our functions are very short, local variables should appear at the top of each function,
**Instance variables**, on the other hand, should be declared at the top of the class. This should not increase the vertical distance of these variables, because in a well-designed class, they are used by many, if not all, of the methods of the class.
There have been many debates over where instance variables should go. In C++ we commonly practiced the so-called scissors rule, which put all the instance variables at the bottom. The common convention in Java, however, is to put them all at the top of the class. I see no reason to follow any other convention. The important thing is for the instance variables to be declared in one well-known place. Everybody should know where to go to see the declarations.
**Dependent Functions**. If one function calls another, they should be vertically close, and the caller should be above the callee, if at all possible. This gives the program a natural flow. If the convention is followed reliably, readers will be able to trust that function definitions will follow shortly after their use.
**Conceptual Affinity**. Certain bits of code want to be near other bits. They have a certain conceptual affinity. The stronger that affinity, the less vertical distance there should be between them.
#### Vertical Ordering
In general we want function call dependencies to point in the downward direction. That is, a function that is called should be below a function that does the calling. This creates a nice flow down the source code module from high level to low level. _(This is the exact opposite of languages like Pascal, C, and C++ that enforce functions to be defined, or at least declared, before they are used)_
### Horizontal Formatting
#### Horizontal Openness and Density
We use horizontal white space to associate things that are strongly related and disassociate things that are more weakly related. Example:
```java
private void measureLine(String line) {
lineCount++;
int lineSize = line.length();
totalChars += lineSize;
lineWidthHistogram.addLine(lineSize, lineCount);
recordWidestLine(lineSize);
}
```
Assignment statements have two distinct and major elements: the left side and the right side. The spaces make that separation obvious.
#### Horizontal Alignment
```java
public class Example implements Base
{
private Socket socket;
private inputStream input;
protected long requestProgress;
public Expediter(Socket s,
inputStream input) {
this.socket = s;
this.input = input;
}
}
```
In modern languages this type of alignment is not useful. The alignment seems to emphasize the wrong things and leads my eye away from the true intent.
```java
public class Example implements Base
{
private Socket socket;
private inputStream input;
protected longrequestProgress;
public Expediter(Socket s, inputStream input) {
this.socket = s;
this.input = input;
}
}
```
This is a better approach.
### Indentation
The indentation it's important because help us to have a visible hierarchy and well defined blocks.
### Team Rules
Every programmer has his own favorite formatting rules, but if he works in a team, then the team rules.
A team of developers should agree upon a single formatting style, and then every member of that team should use that style. We want the software to have a consistent style. We don't want it to appear to have been written by a bunch of disagreeing individuals.
<a name="chapter6">
<h1>Chapter 6 - Objects and Data Structures</h1>
</a>
### Data Abstraction
Hiding implementation is not just a matter of putting a layer of functions between the variables. Hiding implementation is about abstractions! A class does not simply push its variables out through getters and setters. Rather it exposes abstract interfaces that allow its users to manipulate the essence of the data, without having to know its implementation.
### Data/Object Anti-Symmetry
These two examples show the difference between objects and data structures. Objects hide their data behind abstractions and expose functions that operate on that data. Data structure expose their data and have no meaningful functions.
**Procedural Shape**
```java
public class Square {
public Point topLeft;
public double side;
}
public class Rectangle {
public Point topLeft;
public double height;
public double width;
}
public class Circle {
public Point center;
public double radius;
}
public class Geometry {
public final double PI = 3.141592653589793;
public double area(Object shape) throws NoSuchShapeException {
if (shape instanceof Square) {
Square s = (Square)shape;
return s.side * s.side;
}
else if (shape instanceof Rectangle) { Rectangle r = (Rectangle)shape; return r.height * r.width;
}
else if (shape instanceof Circle) {
Circle c = (Circle)shape;
return PI * c.radius * c.radius;
}
throw new NoSuchShapeException();
}
}
```
**Polymorphic Shape**
```java
public class Square implements Shape {
private Point topLeft;
private double side;
public double area() {
return side*side;
}
}
public class Rectangle implements Shape {
private Point topLeft;
private double height;
private double width;
public double area() {
return height * width;
}
}
public class Circle implements Shape {
private Point center;
private double radius;
public final double PI = 3.141592653589793;
public double area() {
return PI * radius * radius;
}
}
```
Again, we see the complimentary nature of these two definitions; they are virtual opposites! This exposes the fundamental dichotomy between objects and data structures:
> Procedural code (code using data structures) makes it easy to add new functions without changing the existing data structures. OO code, on the other hand, makes it easy to add new classes without changing existing functions.
The complement is also true:
> Procedural code makes it hard to add new data structures because all the functions must change. OO code makes it hard to add new functions because all the classes must change.
Mature programmers know that the idea that everything is an object is a myth. Sometimes you really do want simple data structures with procedures operating on them.
### The Law of [Demeter](https://en.wikipedia.org/wiki/Law_of_Demeter)
There is a well-known heuristic called the Law of Demeter that says a module should not know about the innards of the objects it manipulates.
More precisely, the Law of Demeter says that a method `f` of a class `C` should only call the methods of these:
- `C`
- An object created by `f`
- An object passed as an argument to `f`
- An object held in an instance variable of `C`
The method should not invoke methods on objects that are returned by any of the allowed functions. In other words, talk to friends, not to strangers.
### Data Transfer Objects
The quintessential form of a data structure is a class with public variables and no functions. This is sometimes called a data transfer object, or DTO. DTOs are very useful structures, especially when communicating with databases or parsing messages from sockets, and so on. They often become the first in a series of translation stages that convert raw data in a database into objects in the application code.
<a name="chapter7">
<h1>Chapter 7 - Error Handling</h1>
</a>
Many code bases are completely dominated by error handling. When I say dominated, I don't mean that error handling is all that they do. I mean that it is nearly impossible to see what the code does because of all of the scattered error handling. Error handling is important, but if it obscures logic, it's wrong.
### Use Exceptions Rather Than Return Codes
Back in the distant past there were many languages that didn't have exceptions. In those languages the techniques for handling and reporting errors were limited. You either set an error flag or returned an error code that the caller could check
### Write Your Try-Catch-Finally Statement First
In a way, try blocks are like transactions. Your catch has to leave your program in a consistent state, no matter what happens in the try. For this reason it is good practice to start with a try-catch-finally statement when you are writing code that could throw exceptions. This helps you define what the user of that code should expect, no matter what goes wrong with the code that is executed in the try.
### Provide Context with Exceptions
Each exception that you throw should provide enough context to determine the source and location of an error.
Create informative error messages and pass them along with your exceptions. Mention the operation that failed and the type of failure. If you are logging in your application, pass along enough information to be able to log the error in your catch.
### Don't Return Null
If you are tempted to return null from a method, consider throwing an exception or returning a SPECIAL CASE object instead. If you are calling a null-returning method from a third-party API, consider wrapping that method with a method that either throws an exception or returns a special case object.
### Don't Pass Null
Returning null from methods is bad, but passing null into methods is worse. Unless you are working with an API which expects you to pass null, you should avoid passing null in your code whenever possible.
<a name="chapter8">
<h1>Chapter 8 - Boundaries</h1>
</a>
We seldom control all the software in our systems. Sometimes we buy third-party pack- ages or use open source. Other times we depend on teams in our own company to produce components or subsystems for us. Somehow we must cleanly integrate this foreign code with our own.
### Using Third-Party Code
There is a natural tension between the provider of an interface and the user of an interface. Providers of third-party packages and frameworks strive for broad applicability so they can work in many environments and appeal to a wide audience. Users, on the other hand, want an interface that is focused on their particular needs. This tension can cause problems at the boundaries of our systems. Example:
```java
Map sensors = new HashMap();
Sensor s = (Sensor)sensors.get(sensorId);
```
VS
```java
public class Sensors {
private Map sensors = new HashMap();
public Sensor getById(String id) {
return (Sensor) sensors.get(id);
}
//snip
}
```
The first code exposes the casting in the Map, while the second is able to evolve with very little impact on the rest of the application. The casting and type management is handled inside the Sensors class.
The interface is also tailored and constrained to meet the needs of the application. It results in code that is easier to understand and harder to misuse. The Sensors class can enforce design and business rules.
### Exploring and Learning Boundaries
Third-party code helps us get more functionality delivered in less time. Where do we start when we want to utilize some third-party package? It’s not our job to test the third-party code, but it may be in our best interest to write tests for the third-party code we use.
It's a good idea write some test for learn and understand how to use a third-party code. Newkirk calls such tests learning tests.
### Learning Tests Are Better Than Free
The learning tests end up costing nothing. We had to learn the API anyway, and writing those tests was an easy and isolated way to get that knowledge. The learning tests were precise experiments that helped increase our understanding.
Not only are learning tests free, they have a positive return on investment. When there are new releases of the third-party package, we run the learning tests to see whether there are behavioral differences.
### Using Code That Does Not Yet Exist
Some times it's necessary work in a module that will be connected to another module under develop, and we have no idea about how to send the information, because the API had not been designed yet. In this cases it's recommendable create an interface for encapsulate the communication with the pending module. In this way we maintain the control over our module and we can test although the second module isn't available yet.
### Clean Boundaries
Interesting things happen at boundaries. Change is one of those things. Good software designs accommodate change without huge investments and rework. When we use code that is out of our control, special care must be taken to protect our investment and make sure future change is not too costly.
<a name="chapter9">
<h1>Chapter 9 - Unit Tests</h1>
</a>
**T**est
**D**riven
**D**evelopment
### The Three Laws of TDD
- **First Law** You may not write production code until you have written a failing unit test.
- **Second Law** You may not write more of a unit tests than is sufficient to fail, and not comipling is failing.
- **Third Law** You may not write more production code than is sufficient to pass the currently failing tests.
### Clean Tests
If you don't keep your tests clean, you will lose them.
The readability it's very important to keep clean your tests.
### One Assert per test
It's recomendable maintain only one asserts per tests, because this helps to maintain each tests easy to understand.
### Single concept per Test
This rule will help you to keep short functions.
- **Write one test per each concept that you need to verify**
### F.I.R.S.T
- **Fast** Test should be fast.
- **Independient** Test should not depend on each other.
- **Repeatable** Test Should be repeatable in any environment.
- **Self-Validating** Test should have a boolean output. either they pass or fail.
- **Timely** Unit tests should be written just before the production code that makes them pass. If you write tests after the production code, then you may find the production code to be hard to test.
<a name="chapter10">
<h1>Chapter 10 - Classes</h1>
</a>
## Class Organization
### Encapsulation
We like to keep our variables and utility functions small, but we're not fanatic about it. Sometimes we need to make a variable or utility function protected so that it can be accessed by a test.
## Classes Should be Small
- First Rule: Classes should be small
- Second Rule: **Classes should be smaller than the first rule**
### The Single Responsibility Principle
**Classes should have one responsibility - one reason to change**
SRP is one of the more important concept in OO design. It's also one of the simple concepts to understand and adhere to.
### Cohesion
Classes Should have a small number of instance variables. Each of the methods of a class should manipulate one or more of those variables. In general the more variables a method manipulates the more cohesive that method is to its class. A class in which each variable is used by each method is maximally cohesive.
### Maintaining Cohesion Results in Many Small Classes
Just the act of breaking large functions into smaller functions causes a proliferation of classes.
## Organizing for change
For most systems, change is continual. Every change subjects us to the risk that the remainder of the system no longer works as intended. In a clean system we organize our classes so as to reduce the risk of change.
### Isolating from Change
Needs will change, therefore code will change. We learned in OO 101 that there are concrete classes, which contain implementation details (code), and abstract classes, which represent concepts only. A client class depending upon concrete details is at risk when those details change. We can introduce intefaces and abstract classes to help isolate the impact of those details.
<a name="chapter11">
<h1>Chapter 11 - Systems</h1>
</a>
## Separate Constructing a System from using It
_Software Systems should separate the startup process, when the application objects are constructed and the dependencies are "wired" together, from the runtime logic that takes over after startup_
### Separation from main
One way to separate construction from use is simply to move all aspects of construction to `main`, or modules called by `main`, and to design the rest of the system assuming that all objects have been created constructed and wired up appropriately.
The Abstract Factory Pattern is an option for this kind of approach.
### Dependency Injection
A powerful mechanism for separating construction from use is Dependency Injection (DI), the application of Inversion of control (IoC) to dependency management. Inversion of control moves secondary responsibilities from an object to other objects that are dedicated to the purpose, thereby supporting the Single Responsibility Principle. In context of dependency management, an object should not take responsibility for instantiating dependencies itself. Instead, it, should pass this responsibility to another "authoritative" mechanism, thereby inverting the control. Because setup is a global concern, this authoritative mechanism will usually be either the "main"
routine or a special-purpose _container_.
<a name="chapter12">
<h1>Chapter 12 - Emergence</h1>
</a>
According to Kent Beck, a design is "simple" if it follows these rules
- Run all tests
- Contains no duplication
- Expresses the intent of programmers
- Minimizes the number of classes and methods
<a name="chapter13">
<h1>Chapter 13 - Concurrency</h1>
</a>
Concurrence is a decoupling strategy. It helps us decouple what gets fone from when it gets done. In single-threaded applications what and when are so strongly coupled that the state of the entire application can often be determined by looking at the stack backtrace. A programmer who debugs such a system can set a breakpoint, or a sequence of breakpoints, and know the state of the system by which breakpoints are hit.
Decoupling what from when can dramatically improve both the throughput and structures of an application. From a structural point of view the application looks like many lit- tle collaborating computers rather than one big main loop. This can make the system easier to understand and offers some powerful ways to separate concerns.
## Myths and Misconceptions
- Concurrency always improves performance.
Concurrency can sometimes improve performance, but only when there is a lot of wait time that can be shared between multiple threads or multiple processors. Neither situ- ation is trivial.
- Design does not change when writing concurrent programs.
In fact, the design of a concurrent algorithm can be remarkably different from the design of a single-threaded system. The decoupling of what from when usually has a huge effect on the structure of the system.
- Understanding concurrency issues is not important when working with a container such as a Web or EJB container.
In fact, you’d better know just what your container is doing and how to guard against the issues of concurrent update and deadlock described later in this chapter.
Here are a few more balanced sound bites regarding writing concurrent software:
- Concurrency incurs some overhead, both in performance as well as writing additional code.
- Correct concurrency is complex, even for simple problems.
- Concurrency bugs aren’t usually repeatable, so they are often ignored as one-offs instead of the true defects they are.
- Concurrency often requires a fundamental change in design strategy.
#
<a name="chapter14">
<h1>Chapter 14 - Successive Refinement</h1>
</a>
This chapter is a study case. It's recommendable to completely read it to understand more.
<a name="chapter15">
<h1>Chapter 15 - JUnit Internals</h1>
</a>
This chapter analize the JUnit tool. It's recommendable to completely read it to understand more.
<a name="chapter16">
<h1>Chapter 16 - Refactoring SerialDate</h1>
</a>
This chapter is a study case. It's recommendable to completely read it to understand more.
<a name="chapter17">
<h1>Chapter 17 - Smells and Heuristics</h1>
</a>
A reference of code smells from Martin Fowler's _Refactoring_ and Robert C Martin's _Clean Code_.
While clean code comes from discipline and not a list or value system, here is a starting point.
## Comments
#### C1: Inappropriate Information
Reserve comments for technical notes referring to code and design.
#### C2: Obsolete Comment
Update or delete obsolete comments.
#### C3: Redundant Comment
A redundant comment describes something able to sufficiently describe itself.
#### C4: Poorly Written Comment
Comments should be brief, concise, correctly spelled.
#### C5: Commented-Out Code
Ghost code. Delete it.
## Environment
#### E1: Build Requires More Than One Step
Builds should require one command to check out and one command to run.
#### E2: Tests Require More Than One Step
Tests should be run with one button click through an IDE, or else with one command.
## Functions
#### F1: Too Many Arguments
Functions should have no arguments, then one, then two, then three. No more than three.
#### F2: Output Arguments
Arguments are inputs, not outputs. If somethings state must be changed, let it be the state of the called object.
#### F3: Flag Arguments
Eliminate boolean arguments.
#### F4: Dead Function
Discard uncalled methods. This is dead code.
## General
#### G1: Multiple Languages in One Source File
Minimize the number of languages in a source file. Ideally, only one.
#### G2: Obvious Behavior is Unimplemented
The result of a function or class should not be a surprise.
#### G3: Incorrect Behavior at the Boundaries
Write tests for every boundary condition.
#### G4: Overridden Safeties
Overriding safeties and exerting manual control leads to code melt down.
#### G5: Duplication
Practice abstraction on duplicate code. Replace repetitive functions with polymorphism.
#### G6: Code at Wrong Level of Abstraction
Make sure abstracted code is separated into different containers.
#### G7: Base Classes Depending on Their Derivatives
Practice modularity.
#### G8: Too Much Information
Do a lot with a little. Limit the amount of _things_ going on in a class or functions.
#### G9: Dead Code
Delete unexecuted code.
#### G10: Vertical Separation
Define variables and functions close to where they are called.
#### G11: Inconsistency
Choose a convention, and follow it. Remember no surprises.
#### G12: Clutter
Dead code.
#### G13: Artificial Coupling
Favor code that is clear, rather than convenient. Do not group code that favors mental mapping over clearness.
#### G14: Feature Envy
Methods of one class should not be interested with the methods of another class.
#### G15: Selector Arguments
Do not flaunt false arguments at the end of functions.
#### G16: Obscured Intent
Code should not be magic or obscure.
#### G17: Misplaced Responsibility
Use clear function name as waypoints for where to place your code.
#### G18: Inappropriate Static
Make your functions nonstatic.
#### G19: Use Explanatory Variables
Make explanatory variables, and lots of them.
#### G20: Function Names Should Say What They Do
...
#### G21: Understand the Algorithm
Understand how a function works. Passing tests is not enough. Refactoring a function can lead to a better understanding of it.
#### G22: Make Logical Dependencies Physical
Understand what your code is doing.
#### G23: Prefer Polymorphism to If/Else or Switch/Case
Avoid the brute force of switch/case.
#### G24: Follow Standard Conventions
It doesn't matter what your teams convention is. Just that you have on and everyone follows it.
#### G25: Replace Magic Numbers with Named Constants
Stop spelling out numbers.
#### G26: Be Precise
Don't be lazy. Think of possible results, then cover and test them.
#### G27: Structure Over Convention
Design decisions should have a structure rather than a dogma.
#### G28: Encapsulate Conditionals
Make your conditionals more precise.
#### G29: Avoid Negative Conditionals
Negative conditionals take more brain power to understand than a positive.
#### G31: Hidden Temporal Couplings
Use arguments that make temporal coupling explicit.
#### G32: Don’t Be Arbitrary
Your code's structure should communicate the reason for its structure.
#### G33: Encapsulate Boundary Conditions
Avoid leaking +1's and -1's into your code.
#### G34: Functions Should Descend Only One Level of Abstraction
The toughest heuristic to follow. One level of abstraction below the function's described operation can help clarify your code.
#### G35: Keep Configurable Data at High Levels
High level constants are easy to change.
#### G36: Avoid Transitive Navigation
Write shy code. Modules should only know about their neighbors, not their neighbor's neighbors.
## Names
#### N1: Choose Descriptive Names
Choose names that are descriptive and relevant.
#### N2: Choose Names at the Appropriate Level of Abstraction
Think of names that are still clear to the user when used in different programs.
#### N3: Use Standard Nomenclature Where Possible
Use names that express their task.
#### N4: Unambiguous Names
Favor clearness over curtness. A long, expressive name is better than a short, dull one.
#### N5: Use Long Names for Long Scopes
A name's length should relate to its scope.
#### N6: Avoid Encodings
Do not encode names with type or scope information.
#### N7: Names Should Describe Side-Effects
Consider the side-effects of your function, and include that in its name.
## Tests
#### T1: Insufficient Tests
Test everything that can possibly break
#### T2: Use a Coverage Tool!
Use your IDE as a coverage tool.
#### T3: Don’t Skip Trivial Tests
...
#### T4: An Ignored Test is a Question about an Ambiguity
If your test is ignored, the code is brought into question.
#### T5: Test Boundary Conditions
The middle is usually covered. Remember the boundaries.
#### T6: Exhaustively Test Near Bugs
Bugs are rarely alone. When you find one, look nearby for another.
#### T7: Patterns of Failure Are Revealing
Test cases ordered well will reveal patterns of failure.
#### T8: Test Coverage Patterns Can Be Revealing
Similarly, look at the code that is or is not passed in a failure.
#### T9: Tests Should Be Fast
Slow tests won't get run.
# clean-code-javascript
## Table of Contents
1. [Introduction](#introduction)
2. [Variables](#variables)
3. [Functions](#functions)
4. [Objects and Data Structures](#objects-and-data-structures)
5. [Classes](#classes)
6. [SOLID](#solid)
7. [Testing](#testing)
8. [Concurrency](#concurrency)
9. [Error Handling](#error-handling)
10. [Formatting](#formatting)
11. [Comments](#comments)
12. [Translation](#translation)
## Introduction
![Humorous image of software quality estimation as a count of how many expletives
you shout when reading code](https://www.osnews.com/images/comics/wtfm.jpg)
Software engineering principles, from Robert C. Martin's book
[_Clean Code_](https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882),
adapted for JavaScript. This is not a style guide. It's a guide to producing
[readable, reusable, and refactorable](https://github.com/ryanmcdermott/3rs-of-software-architecture) software in JavaScript.
Not every principle herein has to be strictly followed, and even fewer will be
universally agreed upon. These are guidelines and nothing more, but they are
ones codified over many years of collective experience by the authors of
_Clean Code_.
Our craft of software engineering is just a bit over 50 years old, and we are
still learning a lot. When software architecture is as old as architecture
itself, maybe then we will have harder rules to follow. For now, let these
guidelines serve as a touchstone by which to assess the quality of the
JavaScript code that you and your team produce.
One more thing: knowing these won't immediately make you a better software
developer, and working with them for many years doesn't mean you won't make
mistakes. Every piece of code starts as a first draft, like wet clay getting
shaped into its final form. Finally, we chisel away the imperfections when
we review it with our peers. Don't beat yourself up for first drafts that need
improvement. Beat up the code instead!
## **Variables**
### Use meaningful and pronounceable variable names
**Bad:**
```javascript
const yyyymmdstr = moment().format("YYYY/MM/DD");
```
**Good:**
```javascript
const currentDate = moment().format("YYYY/MM/DD");
```
**[⬆ back to top](#table-of-contents)**
### Use the same vocabulary for the same type of variable
**Bad:**
```javascript
getUserInfo();
getClientData();
getCustomerRecord();
```
**Good:**
```javascript
getUser();
```
**[⬆ back to top](#table-of-contents)**
### Use searchable names
We will read more code than we will ever write. It's important that the code we
do write is readable and searchable. By _not_ naming variables that end up
being meaningful for understanding our program, we hurt our readers.
Make your names searchable. Tools like
[buddy.js](https://github.com/danielstjules/buddy.js) and
[ESLint](https://github.com/eslint/eslint/blob/660e0918933e6e7fede26bc675a0763a6b357c94/docs/rules/no-magic-numbers.md)
can help identify unnamed constants.
**Bad:**
```javascript
// What the heck is 86400000 for?
setTimeout(blastOff, 86400000);
```
**Good:**
```javascript
// Declare them as capitalized named constants.
const MILLISECONDS_PER_DAY = 60 * 60 * 24 * 1000; //86400000;
setTimeout(blastOff, MILLISECONDS_PER_DAY);
```
**[⬆ back to top](#table-of-contents)**
### Use explanatory variables
**Bad:**
```javascript
const address = "One Infinite Loop, Cupertino 95014";
const cityZipCodeRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
saveCityZipCode(
address.match(cityZipCodeRegex)[1],
address.match(cityZipCodeRegex)[2]
);
```
**Good:**
```javascript
const address = "One Infinite Loop, Cupertino 95014";
const cityZipCodeRegex = /^[^,\\]+[,\\\s]+(.+?)\s*(\d{5})?$/;
const [_, city, zipCode] = address.match(cityZipCodeRegex) || [];
saveCityZipCode(city, zipCode);
```
**[⬆ back to top](#table-of-contents)**
### Avoid Mental Mapping
Explicit is better than implicit.
**Bad:**
```javascript
const locations = ["Austin", "New York", "San Francisco"];
locations.forEach(l => {
doStuff();
doSomeOtherStuff();
// ...
// ...
// ...
// Wait, what is `l` for again?
dispatch(l);
});
```
**Good:**
```javascript
const locations = ["Austin", "New York", "San Francisco"];
locations.forEach(location => {
doStuff();
doSomeOtherStuff();
// ...
// ...
// ...
dispatch(location);
});
```
**[⬆ back to top](#table-of-contents)**
### Don't add unneeded context
If your class/object name tells you something, don't repeat that in your
variable name.
**Bad:**
```javascript
const Car = {
carMake: "Honda",
carModel: "Accord",
carColor: "Blue"
};
function paintCar(car, color) {
car.carColor = color;
}
```
**Good:**
```javascript
const Car = {
make: "Honda",
model: "Accord",
color: "Blue"
};
function paintCar(car, color) {
car.color = color;
}
```
**[⬆ back to top](#table-of-contents)**
### Use default parameters instead of short circuiting or conditionals
Default parameters are often cleaner than short circuiting. Be aware that if you
use them, your function will only provide default values for `undefined`
arguments. Other "falsy" values such as `''`, `""`, `false`, `null`, `0`, and
`NaN`, will not be replaced by a default value.
**Bad:**
```javascript
function createMicrobrewery(name) {
const breweryName = name || "Hipster Brew Co.";
// ...
}
```
**Good:**
```javascript
function createMicrobrewery(name = "Hipster Brew Co.") {
// ...
}
```
**[⬆ back to top](#table-of-contents)**
## **Functions**
### Function arguments (2 or fewer ideally)
Limiting the amount of function parameters is incredibly important because it
makes testing your function easier. Having more than three leads to a
combinatorial explosion where you have to test tons of different cases with
each separate argument.
One or two arguments is the ideal case, and three should be avoided if possible.
Anything more than that should be consolidated. Usually, if you have
more than two arguments then your function is trying to do too much. In cases
where it's not, most of the time a higher-level object will suffice as an
argument.
Since JavaScript allows you to make objects on the fly, without a lot of class
boilerplate, you can use an object if you are finding yourself needing a
lot of arguments.
To make it obvious what properties the function expects, you can use the ES2015/ES6
destructuring syntax. This has a few advantages:
1. When someone looks at the function signature, it's immediately clear what
properties are being used.
2. It can be used to simulate named parameters.
3. Destructuring also clones the specified primitive values of the argument
object passed into the function. This can help prevent side effects. Note:
objects and arrays that are destructured from the argument object are NOT
cloned.
4. Linters can warn you about unused properties, which would be impossible
without destructuring.
**Bad:**
```javascript
function createMenu(title, body, buttonText, cancellable) {
// ...
}
createMenu("Foo", "Bar", "Baz", true);
```
**Good:**
```javascript
function createMenu({ title, body, buttonText, cancellable }) {
// ...
}
createMenu({
title: "Foo",
body: "Bar",
buttonText: "Baz",
cancellable: true
});
```
**[⬆ back to top](#table-of-contents)**
### Functions should do one thing
This is by far the most important rule in software engineering. When functions
do more than one thing, they are harder to compose, test, and reason about.
When you can isolate a function to just one action, it can be refactored
easily and your code will read much cleaner. If you take nothing else away from
this guide other than this, you'll be ahead of many developers.
**Bad:**
```javascript
function emailClients(clients) {
clients.forEach(client => {
const clientRecord = database.lookup(client);
if (clientRecord.isActive()) {
email(client);
}
});
}
```
**Good:**
```javascript
function emailActiveClients(clients) {
clients.filter(isActiveClient).forEach(email);
}
function isActiveClient(client) {
const clientRecord = database.lookup(client);
return clientRecord.isActive();
}
```
**[⬆ back to top](#table-of-contents)**
### Function names should say what they do
**Bad:**
```javascript
function addToDate(date, month) {
// ...
}
const date = new Date();
// It's hard to tell from the function name what is added
addToDate(date, 1);
```
**Good:**
```javascript
function addMonthToDate(month, date) {
// ...
}
const date = new Date();
addMonthToDate(1, date);
```
**[⬆ back to top](#table-of-contents)**
### Functions should only be one level of abstraction
When you have more than one level of abstraction your function is usually
doing too much. Splitting up functions leads to reusability and easier
testing.
**Bad:**
```javascript
function parseBetterJSAlternative(code) {
const REGEXES = [
// ...
];
const statements = code.split(" ");
const tokens = [];
REGEXES.forEach(REGEX => {
statements.forEach(statement => {
// ...
});
});
const ast = [];
tokens.forEach(token => {
// lex...
});
ast.forEach(node => {
// parse...
});
}
```
**Good:**
```javascript
function parseBetterJSAlternative(code) {
const tokens = tokenize(code);
const syntaxTree = parse(tokens);
syntaxTree.forEach(node => {
// parse...
});
}
function tokenize(code) {
const REGEXES = [
// ...
];
const statements = code.split(" ");
const tokens = [];
REGEXES.forEach(REGEX => {
statements.forEach(statement => {
tokens.push(/* ... */);
});
});
return tokens;
}
function parse(tokens) {
const syntaxTree = [];
tokens.forEach(token => {
syntaxTree.push(/* ... */);
});
return syntaxTree;
}
```
**[⬆ back to top](#table-of-contents)**
### Remove duplicate code
Do your absolute best to avoid duplicate code. Duplicate code is bad because it
means that there's more than one place to alter something if you need to change
some logic.
Imagine if you run a restaurant and you keep track of your inventory: all your
tomatoes, onions, garlic, spices, etc. If you have multiple lists that
you keep this on, then all have to be updated when you serve a dish with
tomatoes in them. If you only have one list, there's only one place to update!
Oftentimes you have duplicate code because you have two or more slightly
different things, that share a lot in common, but their differences force you
to have two or more separate functions that do much of the same things. Removing
duplicate code means creating an abstraction that can handle this set of
different things with just one function/module/class.
Getting the abstraction right is critical, that's why you should follow the
SOLID principles laid out in the _Classes_ section. Bad abstractions can be
worse than duplicate code, so be careful! Having said this, if you can make
a good abstraction, do it! Don't repeat yourself, otherwise you'll find yourself
updating multiple places anytime you want to change one thing.
**Bad:**
```javascript
function showDeveloperList(developers) {
developers.forEach(developer => {
const expectedSalary = developer.calculateExpectedSalary();
const experience = developer.getExperience();
const githubLink = developer.getGithubLink();
const data = {
expectedSalary,
experience,
githubLink
};
render(data);
});
}
function showManagerList(managers) {
managers.forEach(manager => {
const expectedSalary = manager.calculateExpectedSalary();
const experience = manager.getExperience();
const portfolio = manager.getMBAProjects();
const data = {
expectedSalary,
experience,
portfolio
};
render(data);
});
}
```
**Good:**
```javascript
function showEmployeeList(employees) {
employees.forEach(employee => {
const expectedSalary = employee.calculateExpectedSalary();
const experience = employee.getExperience();
const data = {
expectedSalary,
experience
};
switch (employee.type) {
case "manager":
data.portfolio = employee.getMBAProjects();
break;
case "developer":
data.githubLink = employee.getGithubLink();
break;
}
render(data);
});
}
```
**[⬆ back to top](#table-of-contents)**
### Set default objects with Object.assign
**Bad:**
```javascript
const menuConfig = {
title: null,
body: "Bar",
buttonText: null,
cancellable: true
};
function createMenu(config) {
config.title = config.title || "Foo";
config.body = config.body || "Bar";
config.buttonText = config.buttonText || "Baz";
config.cancellable =
config.cancellable !== undefined ? config.cancellable : true;
}
createMenu(menuConfig);
```
**Good:**
```javascript
const menuConfig = {
title: "Order",
// User did not include 'body' key
buttonText: "Send",
cancellable: true
};
function createMenu(config) {
let finalConfig = Object.assign(
{
title: "Foo",
body: "Bar",
buttonText: "Baz",
cancellable: true
},
config
);
return finalConfig
// config now equals: {title: "Order", body: "Bar", buttonText: "Send", cancellable: true}
// ...
}
createMenu(menuConfig);
```
**[⬆ back to top](#table-of-contents)**
### Don't use flags as function parameters
Flags tell your user that this function does more than one thing. Functions should do one thing. Split out your functions if they are following different code paths based on a boolean.
**Bad:**
```javascript
function createFile(name, temp) {
if (temp) {
fs.create(`./temp/${name}`);
} else {
fs.create(name);
}
}
```
**Good:**
```javascript
function createFile(name) {
fs.create(name);
}
function createTempFile(name) {
createFile(`./temp/${name}`);
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid Side Effects (part 1)
A function produces a side effect if it does anything other than take a value in
and return another value or values. A side effect could be writing to a file,
modifying some global variable, or accidentally wiring all your money to a
stranger.
Now, you do need to have side effects in a program on occasion. Like the previous
example, you might need to write to a file. What you want to do is to
centralize where you are doing this. Don't have several functions and classes
that write to a particular file. Have one service that does it. One and only one.
The main point is to avoid common pitfalls like sharing state between objects
without any structure, using mutable data types that can be written to by anything,
and not centralizing where your side effects occur. If you can do this, you will
be happier than the vast majority of other programmers.
**Bad:**
```javascript
// Global variable referenced by following function.
// If we had another function that used this name, now it'd be an array and it could break it.
let name = "Ryan McDermott";
function splitIntoFirstAndLastName() {
name = name.split(" ");
}
splitIntoFirstAndLastName();
console.log(name); // ['Ryan', 'McDermott'];
```
**Good:**
```javascript
function splitIntoFirstAndLastName(name) {
return name.split(" ");
}
const name = "Ryan McDermott";
const newName = splitIntoFirstAndLastName(name);
console.log(name); // 'Ryan McDermott';
console.log(newName); // ['Ryan', 'McDermott'];
```
**[⬆ back to top](#table-of-contents)**
### Avoid Side Effects (part 2)
In JavaScript, some values are unchangeable (immutable) and some are changeable
(mutable). Objects and arrays are two kinds of mutable values so it's important
to handle them carefully when they're passed as parameters to a function. A
JavaScript function can change an object's properties or alter the contents of
an array which could easily cause bugs elsewhere.
Suppose there's a function that accepts an array parameter representing a
shopping cart. If the function makes a change in that shopping cart array -
by adding an item to purchase, for example - then any other function that
uses that same `cart` array will be affected by this addition. That may be
great, however it could also be bad. Let's imagine a bad situation:
The user clicks the "Purchase" button which calls a `purchase` function that
spawns a network request and sends the `cart` array to the server. Because
of a bad network connection, the `purchase` function has to keep retrying the
request. Now, what if in the meantime the user accidentally clicks an "Add to Cart"
button on an item they don't actually want before the network request begins?
If that happens and the network request begins, then that purchase function
will send the accidentally added item because the `cart` array was modified.
A great solution would be for the `addItemToCart` function to always clone the
`cart`, edit it, and return the clone. This would ensure that functions that are still
using the old shopping cart wouldn't be affected by the changes.
Two caveats to mention to this approach:
1. There might be cases where you actually want to modify the input object,
but when you adopt this programming practice you will find that those cases
are pretty rare. Most things can be refactored to have no side effects!
2. Cloning big objects can be very expensive in terms of performance. Luckily,
this isn't a big issue in practice because there are
[great libraries](https://facebook.github.io/immutable-js/) that allow
this kind of programming approach to be fast and not as memory intensive as
it would be for you to manually clone objects and arrays.
**Bad:**
```javascript
const addItemToCart = (cart, item) => {
cart.push({ item, date: Date.now() });
};
```
**Good:**
```javascript
const addItemToCart = (cart, item) => {
return [...cart, { item, date: Date.now() }];
};
```
**[⬆ back to top](#table-of-contents)**
### Don't write to global functions
Polluting globals is a bad practice in JavaScript because you could clash with another
library and the user of your API would be none-the-wiser until they get an
exception in production. Let's think about an example: what if you wanted to
extend JavaScript's native Array method to have a `diff` method that could
show the difference between two arrays? You could write your new function
to the `Array.prototype`, but it could clash with another library that tried
to do the same thing. What if that other library was just using `diff` to find
the difference between the first and last elements of an array? This is why it
would be much better to just use ES2015/ES6 classes and simply extend the `Array` global.
**Bad:**
```javascript
Array.prototype.diff = function diff(comparisonArray) {
const hash = new Set(comparisonArray);
return this.filter(elem => !hash.has(elem));
};
```
**Good:**
```javascript
class SuperArray extends Array {
diff(comparisonArray) {
const hash = new Set(comparisonArray);
return this.filter(elem => !hash.has(elem));
}
}
```
**[⬆ back to top](#table-of-contents)**
### Favor functional programming over imperative programming
JavaScript isn't a functional language in the way that Haskell is, but it has
a functional flavor to it. Functional languages can be cleaner and easier to test.
Favor this style of programming when you can.
**Bad:**
```javascript
const programmerOutput = [
{
name: "Uncle Bobby",
linesOfCode: 500
},
{
name: "Suzie Q",
linesOfCode: 1500
},
{
name: "Jimmy Gosling",
linesOfCode: 150
},
{
name: "Gracie Hopper",
linesOfCode: 1000
}
];
let totalOutput = 0;
for (let i = 0; i < programmerOutput.length; i++) {
totalOutput += programmerOutput[i].linesOfCode;
}
```
**Good:**
```javascript
const programmerOutput = [
{
name: "Uncle Bobby",
linesOfCode: 500
},
{
name: "Suzie Q",
linesOfCode: 1500
},
{
name: "Jimmy Gosling",
linesOfCode: 150
},
{
name: "Gracie Hopper",
linesOfCode: 1000
}
];
const totalOutput = programmerOutput.reduce(
(totalLines, output) => totalLines + output.linesOfCode,
0
);
```
**[⬆ back to top](#table-of-contents)**
### Encapsulate conditionals
**Bad:**
```javascript
if (fsm.state === "fetching" && isEmpty(listNode)) {
// ...
}
```
**Good:**
```javascript
function shouldShowSpinner(fsm, listNode) {
return fsm.state === "fetching" && isEmpty(listNode);
}
if (shouldShowSpinner(fsmInstance, listNodeInstance)) {
// ...
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid negative conditionals
**Bad:**
```javascript
function isDOMNodeNotPresent(node) {
// ...
}
if (!isDOMNodeNotPresent(node)) {
// ...
}
```
**Good:**
```javascript
function isDOMNodePresent(node) {
// ...
}
if (isDOMNodePresent(node)) {
// ...
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid conditionals
This seems like an impossible task. Upon first hearing this, most people say,
"how am I supposed to do anything without an `if` statement?" The answer is that
you can use polymorphism to achieve the same task in many cases. The second
question is usually, "well that's great but why would I want to do that?" The
answer is a previous clean code concept we learned: a function should only do
one thing. When you have classes and functions that have `if` statements, you
are telling your user that your function does more than one thing. Remember,
just do one thing.
**Bad:**
```javascript
class Airplane {
// ...
getCruisingAltitude() {
switch (this.type) {
case "777":
return this.getMaxAltitude() - this.getPassengerCount();
case "Air Force One":
return this.getMaxAltitude();
case "Cessna":
return this.getMaxAltitude() - this.getFuelExpenditure();
}
}
}
```
**Good:**
```javascript
class Airplane {
// ...
}
class Boeing777 extends Airplane {
// ...
getCruisingAltitude() {
return this.getMaxAltitude() - this.getPassengerCount();
}
}
class AirForceOne extends Airplane {
// ...
getCruisingAltitude() {
return this.getMaxAltitude();
}
}
class Cessna extends Airplane {
// ...
getCruisingAltitude() {
return this.getMaxAltitude() - this.getFuelExpenditure();
}
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid type-checking (part 1)
JavaScript is untyped, which means your functions can take any type of argument.
Sometimes you are bitten by this freedom and it becomes tempting to do
type-checking in your functions. There are many ways to avoid having to do this.
The first thing to consider is consistent APIs.
**Bad:**
```javascript
function travelToTexas(vehicle) {
if (vehicle instanceof Bicycle) {
vehicle.pedal(this.currentLocation, new Location("texas"));
} else if (vehicle instanceof Car) {
vehicle.drive(this.currentLocation, new Location("texas"));
}
}
```
**Good:**
```javascript
function travelToTexas(vehicle) {
vehicle.move(this.currentLocation, new Location("texas"));
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid type-checking (part 2)
If you are working with basic primitive values like strings and integers,
and you can't use polymorphism but you still feel the need to type-check,
you should consider using TypeScript. It is an excellent alternative to normal
JavaScript, as it provides you with static typing on top of standard JavaScript
syntax. The problem with manually type-checking normal JavaScript is that
doing it well requires so much extra verbiage that the faux "type-safety" you get
doesn't make up for the lost readability. Keep your JavaScript clean, write
good tests, and have good code reviews. Otherwise, do all of that but with
TypeScript (which, like I said, is a great alternative!).
**Bad:**
```javascript
function combine(val1, val2) {
if (
(typeof val1 === "number" && typeof val2 === "number") ||
(typeof val1 === "string" && typeof val2 === "string")
) {
return val1 + val2;
}
throw new Error("Must be of type String or Number");
}
```
**Good:**
```javascript
function combine(val1, val2) {
return val1 + val2;
}
```
**[⬆ back to top](#table-of-contents)**
### Don't over-optimize
Modern browsers do a lot of optimization under-the-hood at runtime. A lot of
times, if you are optimizing then you are just wasting your time. [There are good
resources](https://github.com/petkaantonov/bluebird/wiki/Optimization-killers)
for seeing where optimization is lacking. Target those in the meantime, until
they are fixed if they can be.
**Bad:**
```javascript
// On old browsers, each iteration with uncached `list.length` would be costly
// because of `list.length` recomputation. In modern browsers, this is optimized.
for (let i = 0, len = list.length; i < len; i++) {
// ...
}
```
**Good:**
```javascript
for (let i = 0; i < list.length; i++) {
// ...
}
```
**[⬆ back to top](#table-of-contents)**
### Remove dead code
Dead code is just as bad as duplicate code. There's no reason to keep it in
your codebase. If it's not being called, get rid of it! It will still be safe
in your version history if you still need it.
**Bad:**
```javascript
function oldRequestModule(url) {
// ...
}
function newRequestModule(url) {
// ...
}
const req = newRequestModule;
inventoryTracker("apples", req, "www.inventory-awesome.io");
```
**Good:**
```javascript
function newRequestModule(url) {
// ...
}
const req = newRequestModule;
inventoryTracker("apples", req, "www.inventory-awesome.io");
```
**[⬆ back to top](#table-of-contents)**
## **Objects and Data Structures**
### Use getters and setters
Using getters and setters to access data on objects could be better than simply
looking for a property on an object. "Why?" you might ask. Well, here's an
unorganized list of reasons why:
- When you want to do more beyond getting an object property, you don't have
to look up and change every accessor in your codebase.
- Makes adding validation simple when doing a `set`.
- Encapsulates the internal representation.
- Easy to add logging and error handling when getting and setting.
- You can lazy load your object's properties, let's say getting it from a
server.
**Bad:**
```javascript
function makeBankAccount() {
// ...
return {
balance: 0
// ...
};
}
const account = makeBankAccount();
account.balance = 100;
```
**Good:**
```javascript
function makeBankAccount() {
// this one is private
let balance = 0;
// a "getter", made public via the returned object below
function getBalance() {
return balance;
}
// a "setter", made public via the returned object below
function setBalance(amount) {
// ... validate before updating the balance
balance = amount;
}
return {
// ...
getBalance,
setBalance
};
}
const account = makeBankAccount();
account.setBalance(100);
```
**[⬆ back to top](#table-of-contents)**
### Make objects have private members
This can be accomplished through closures (for ES5 and below).
**Bad:**
```javascript
const Employee = function(name) {
this.name = name;
};
Employee.prototype.getName = function getName() {
return this.name;
};
const employee = new Employee("John Doe");
console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe
delete employee.name;
console.log(`Employee name: ${employee.getName()}`); // Employee name: undefined
```
**Good:**
```javascript
function makeEmployee(name) {
return {
getName() {
return name;
}
};
}
const employee = makeEmployee("John Doe");
console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe
delete employee.name;
console.log(`Employee name: ${employee.getName()}`); // Employee name: John Doe
```
**[⬆ back to top](#table-of-contents)**
## **Classes**
### Prefer ES2015/ES6 classes over ES5 plain functions
It's very difficult to get readable class inheritance, construction, and method
definitions for classical ES5 classes. If you need inheritance (and be aware
that you might not), then prefer ES2015/ES6 classes. However, prefer small functions over
classes until you find yourself needing larger and more complex objects.
**Bad:**
```javascript
const Animal = function(age) {
if (!(this instanceof Animal)) {
throw new Error("Instantiate Animal with `new`");
}
this.age = age;
};
Animal.prototype.move = function move() {};
const Mammal = function(age, furColor) {
if (!(this instanceof Mammal)) {
throw new Error("Instantiate Mammal with `new`");
}
Animal.call(this, age);
this.furColor = furColor;
};
Mammal.prototype = Object.create(Animal.prototype);
Mammal.prototype.constructor = Mammal;
Mammal.prototype.liveBirth = function liveBirth() {};
const Human = function(age, furColor, languageSpoken) {
if (!(this instanceof Human)) {
throw new Error("Instantiate Human with `new`");
}
Mammal.call(this, age, furColor);
this.languageSpoken = languageSpoken;
};
Human.prototype = Object.create(Mammal.prototype);
Human.prototype.constructor = Human;
Human.prototype.speak = function speak() {};
```
**Good:**
```javascript
class Animal {
constructor(age) {
this.age = age;
}
move() {
/* ... */
}
}
class Mammal extends Animal {
constructor(age, furColor) {
super(age);
this.furColor = furColor;
}
liveBirth() {
/* ... */
}
}
class Human extends Mammal {
constructor(age, furColor, languageSpoken) {
super(age, furColor);
this.languageSpoken = languageSpoken;
}
speak() {
/* ... */
}
}
```
**[⬆ back to top](#table-of-contents)**
### Use method chaining
This pattern is very useful in JavaScript and you see it in many libraries such
as jQuery and Lodash. It allows your code to be expressive, and less verbose.
For that reason, I say, use method chaining and take a look at how clean your code
will be. In your class functions, simply return `this` at the end of every function,
and you can chain further class methods onto it.
**Bad:**
```javascript
class Car {
constructor(make, model, color) {
this.make = make;
this.model = model;
this.color = color;
}
setMake(make) {
this.make = make;
}
setModel(model) {
this.model = model;
}
setColor(color) {
this.color = color;
}
save() {
console.log(this.make, this.model, this.color);
}
}
const car = new Car("Ford", "F-150", "red");
car.setColor("pink");
car.save();
```
**Good:**
```javascript
class Car {
constructor(make, model, color) {
this.make = make;
this.model = model;
this.color = color;
}
setMake(make) {
this.make = make;
// NOTE: Returning this for chaining
return this;
}
setModel(model) {
this.model = model;
// NOTE: Returning this for chaining
return this;
}
setColor(color) {
this.color = color;
// NOTE: Returning this for chaining
return this;
}
save() {
console.log(this.make, this.model, this.color);
// NOTE: Returning this for chaining
return this;
}
}
const car = new Car("Ford", "F-150", "red").setColor("pink").save();
```
**[⬆ back to top](#table-of-contents)**
### Prefer composition over inheritance
As stated famously in [_Design Patterns_](https://en.wikipedia.org/wiki/Design_Patterns) by the Gang of Four,
you should prefer composition over inheritance where you can. There are lots of
good reasons to use inheritance and lots of good reasons to use composition.
The main point for this maxim is that if your mind instinctively goes for
inheritance, try to think if composition could model your problem better. In some
cases it can.
You might be wondering then, "when should I use inheritance?" It
depends on your problem at hand, but this is a decent list of when inheritance
makes more sense than composition:
1. Your inheritance represents an "is-a" relationship and not a "has-a"
relationship (Human->Animal vs. User->UserDetails).
2. You can reuse code from the base classes (Humans can move like all animals).
3. You want to make global changes to derived classes by changing a base class.
(Change the caloric expenditure of all animals when they move).
**Bad:**
```javascript
class Employee {
constructor(name, email) {
this.name = name;
this.email = email;
}
// ...
}
// Bad because Employees "have" tax data. EmployeeTaxData is not a type of Employee
class EmployeeTaxData extends Employee {
constructor(ssn, salary) {
super();
this.ssn = ssn;
this.salary = salary;
}
// ...
}
```
**Good:**
```javascript
class EmployeeTaxData {
constructor(ssn, salary) {
this.ssn = ssn;
this.salary = salary;
}
// ...
}
class Employee {
constructor(name, email) {
this.name = name;
this.email = email;
}
setTaxData(ssn, salary) {
this.taxData = new EmployeeTaxData(ssn, salary);
}
// ...
}
```
**[⬆ back to top](#table-of-contents)**
## **SOLID**
### Single Responsibility Principle (SRP)
As stated in Clean Code, "There should never be more than one reason for a class
to change". It's tempting to jam-pack a class with a lot of functionality, like
when you can only take one suitcase on your flight. The issue with this is
that your class won't be conceptually cohesive and it will give it many reasons
to change. Minimizing the amount of times you need to change a class is important.
It's important because if too much functionality is in one class and you modify
a piece of it, it can be difficult to understand how that will affect other
dependent modules in your codebase.
**Bad:**
```javascript
class UserSettings {
constructor(user) {
this.user = user;
}
changeSettings(settings) {
if (this.verifyCredentials()) {
// ...
}
}
verifyCredentials() {
// ...
}
}
```
**Good:**
```javascript
class UserAuth {
constructor(user) {
this.user = user;
}
verifyCredentials() {
// ...
}
}
class UserSettings {
constructor(user) {
this.user = user;
this.auth = new UserAuth(user);
}
changeSettings(settings) {
if (this.auth.verifyCredentials()) {
// ...
}
}
}
```
**[⬆ back to top](#table-of-contents)**
### Open/Closed Principle (OCP)
As stated by Bertrand Meyer, "software entities (classes, modules, functions,
etc.) should be open for extension, but closed for modification." What does that
mean though? This principle basically states that you should allow users to
add new functionalities without changing existing code.
**Bad:**
```javascript
class AjaxAdapter extends Adapter {
constructor() {
super();
this.name = "ajaxAdapter";
}
}
class NodeAdapter extends Adapter {
constructor() {
super();
this.name = "nodeAdapter";
}
}
class HttpRequester {
constructor(adapter) {
this.adapter = adapter;
}
fetch(url) {
if (this.adapter.name === "ajaxAdapter") {
return makeAjaxCall(url).then(response => {
// transform response and return
});
} else if (this.adapter.name === "nodeAdapter") {
return makeHttpCall(url).then(response => {
// transform response and return
});
}
}
}
function makeAjaxCall(url) {
// request and return promise
}
function makeHttpCall(url) {
// request and return promise
}
```
**Good:**
```javascript
class AjaxAdapter extends Adapter {
constructor() {
super();
this.name = "ajaxAdapter";
}
request(url) {
// request and return promise
}
}
class NodeAdapter extends Adapter {
constructor() {
super();
this.name = "nodeAdapter";
}
request(url) {
// request and return promise
}
}
class HttpRequester {
constructor(adapter) {
this.adapter = adapter;
}
fetch(url) {
return this.adapter.request(url).then(response => {
// transform response and return
});
}
}
```
**[⬆ back to top](#table-of-contents)**
### Liskov Substitution Principle (LSP)
This is a scary term for a very simple concept. It's formally defined as "If S
is a subtype of T, then objects of type T may be replaced with objects of type S
(i.e., objects of type S may substitute objects of type T) without altering any
of the desirable properties of that program (correctness, task performed,
etc.)." That's an even scarier definition.
The best explanation for this is if you have a parent class and a child class,
then the base class and child class can be used interchangeably without getting
incorrect results. This might still be confusing, so let's take a look at the
classic Square-Rectangle example. Mathematically, a square is a rectangle, but
if you model it using the "is-a" relationship via inheritance, you quickly
get into trouble.
**Bad:**
```javascript
class Rectangle {
constructor() {
this.width = 0;
this.height = 0;
}
setColor(color) {
// ...
}
render(area) {
// ...
}
setWidth(width) {
this.width = width;
}
setHeight(height) {
this.height = height;
}
getArea() {
return this.width * this.height;
}
}
class Square extends Rectangle {
setWidth(width) {
this.width = width;
this.height = width;
}
setHeight(height) {
this.width = height;
this.height = height;
}
}
function renderLargeRectangles(rectangles) {
rectangles.forEach(rectangle => {
rectangle.setWidth(4);
rectangle.setHeight(5);
const area = rectangle.getArea(); // BAD: Returns 25 for Square. Should be 20.
rectangle.render(area);
});
}
const rectangles = [new Rectangle(), new Rectangle(), new Square()];
renderLargeRectangles(rectangles);
```
**Good:**
```javascript
class Shape {
setColor(color) {
// ...
}
render(area) {
// ...
}
}
class Rectangle extends Shape {
constructor(width, height) {
super();
this.width = width;
this.height = height;
}
getArea() {
return this.width * this.height;
}
}
class Square extends Shape {
constructor(length) {
super();
this.length = length;
}
getArea() {
return this.length * this.length;
}
}
function renderLargeShapes(shapes) {
shapes.forEach(shape => {
const area = shape.getArea();
shape.render(area);
});
}
const shapes = [new Rectangle(4, 5), new Rectangle(4, 5), new Square(5)];
renderLargeShapes(shapes);
```
**[⬆ back to top](#table-of-contents)**
### Interface Segregation Principle (ISP)
JavaScript doesn't have interfaces so this principle doesn't apply as strictly
as others. However, it's important and relevant even with JavaScript's lack of
type system.
ISP states that "Clients should not be forced to depend upon interfaces that
they do not use." Interfaces are implicit contracts in JavaScript because of
duck typing.
A good example to look at that demonstrates this principle in JavaScript is for
classes that require large settings objects. Not requiring clients to setup
huge amounts of options is beneficial, because most of the time they won't need
all of the settings. Making them optional helps prevent having a
"fat interface".
**Bad:**
```javascript
class DOMTraverser {
constructor(settings) {
this.settings = settings;
this.setup();
}
setup() {
this.rootNode = this.settings.rootNode;
this.settings.animationModule.setup();
}
traverse() {
// ...
}
}
const $ = new DOMTraverser({
rootNode: document.getElementsByTagName("body"),
animationModule() {} // Most of the time, we won't need to animate when traversing.
// ...
});
```
**Good:**
```javascript
class DOMTraverser {
constructor(settings) {
this.settings = settings;
this.options = settings.options;
this.setup();
}
setup() {
this.rootNode = this.settings.rootNode;
this.setupOptions();
}
setupOptions() {
if (this.options.animationModule) {
// ...
}
}
traverse() {
// ...
}
}
const $ = new DOMTraverser({
rootNode: document.getElementsByTagName("body"),
options: {
animationModule() {}
}
});
```
**[⬆ back to top](#table-of-contents)**
### Dependency Inversion Principle (DIP)
This principle states two essential things:
1. High-level modules should not depend on low-level modules. Both should
depend on abstractions.
2. Abstractions should not depend upon details. Details should depend on
abstractions.
This can be hard to understand at first, but if you've worked with AngularJS,
you've seen an implementation of this principle in the form of Dependency
Injection (DI). While they are not identical concepts, DIP keeps high-level
modules from knowing the details of its low-level modules and setting them up.
It can accomplish this through DI. A huge benefit of this is that it reduces
the coupling between modules. Coupling is a very bad development pattern because
it makes your code hard to refactor.
As stated previously, JavaScript doesn't have interfaces so the abstractions
that are depended upon are implicit contracts. That is to say, the methods
and properties that an object/class exposes to another object/class. In the
example below, the implicit contract is that any Request module for an
`InventoryTracker` will have a `requestItems` method.
**Bad:**
```javascript
class InventoryRequester {
constructor() {
this.REQ_METHODS = ["HTTP"];
}
requestItem(item) {
// ...
}
}
class InventoryTracker {
constructor(items) {
this.items = items;
// BAD: We have created a dependency on a specific request implementation.
// We should just have requestItems depend on a request method: `request`
this.requester = new InventoryRequester();
}
requestItems() {
this.items.forEach(item => {
this.requester.requestItem(item);
});
}
}
const inventoryTracker = new InventoryTracker(["apples", "bananas"]);
inventoryTracker.requestItems();
```
**Good:**
```javascript
class InventoryTracker {
constructor(items, requester) {
this.items = items;
this.requester = requester;
}
requestItems() {
this.items.forEach(item => {
this.requester.requestItem(item);
});
}
}
class InventoryRequesterV1 {
constructor() {
this.REQ_METHODS = ["HTTP"];
}
requestItem(item) {
// ...
}
}
class InventoryRequesterV2 {
constructor() {
this.REQ_METHODS = ["WS"];
}
requestItem(item) {
// ...
}
}
// By constructing our dependencies externally and injecting them, we can easily
// substitute our request module for a fancy new one that uses WebSockets.
const inventoryTracker = new InventoryTracker(
["apples", "bananas"],
new InventoryRequesterV2()
);
inventoryTracker.requestItems();
```
**[⬆ back to top](#table-of-contents)**
## **Testing**
Testing is more important than shipping. If you have no tests or an
inadequate amount, then every time you ship code you won't be sure that you
didn't break anything. Deciding on what constitutes an adequate amount is up
to your team, but having 100% coverage (all statements and branches) is how
you achieve very high confidence and developer peace of mind. This means that
in addition to having a great testing framework, you also need to use a
[good coverage tool](https://gotwarlost.github.io/istanbul/).
There's no excuse to not write tests. There are [plenty of good JS test frameworks](https://jstherightway.org/#testing-tools), so find one that your team prefers.
When you find one that works for your team, then aim to always write tests
for every new feature/module you introduce. If your preferred method is
Test Driven Development (TDD), that is great, but the main point is to just
make sure you are reaching your coverage goals before launching any feature,
or refactoring an existing one.
### Single concept per test
**Bad:**
```javascript
import assert from "assert";
describe("MomentJS", () => {
it("handles date boundaries", () => {
let date;
date = new MomentJS("1/1/2015");
date.addDays(30);
assert.equal("1/31/2015", date);
date = new MomentJS("2/1/2016");
date.addDays(28);
assert.equal("02/29/2016", date);
date = new MomentJS("2/1/2015");
date.addDays(28);
assert.equal("03/01/2015", date);
});
});
```
**Good:**
```javascript
import assert from "assert";
describe("MomentJS", () => {
it("handles 30-day months", () => {
const date = new MomentJS("1/1/2015");
date.addDays(30);
assert.equal("1/31/2015", date);
});
it("handles leap year", () => {
const date = new MomentJS("2/1/2016");
date.addDays(28);
assert.equal("02/29/2016", date);
});
it("handles non-leap year", () => {
const date = new MomentJS("2/1/2015");
date.addDays(28);
assert.equal("03/01/2015", date);
});
});
```
**[⬆ back to top](#table-of-contents)**
## **Concurrency**
### Use Promises, not callbacks
Callbacks aren't clean, and they cause excessive amounts of nesting. With ES2015/ES6,
Promises are a built-in global type. Use them!
**Bad:**
```javascript
import { get } from "request";
import { writeFile } from "fs";
get(
"https://en.wikipedia.org/wiki/Robert_Cecil_Martin",
(requestErr, response, body) => {
if (requestErr) {
console.error(requestErr);
} else {
writeFile("article.html", body, writeErr => {
if (writeErr) {
console.error(writeErr);
} else {
console.log("File written");
}
});
}
}
);
```
**Good:**
```javascript
import { get } from "request-promise";
import { writeFile } from "fs-extra";
get("https://en.wikipedia.org/wiki/Robert_Cecil_Martin")
.then(body => {
return writeFile("article.html", body);
})
.then(() => {
console.log("File written");
})
.catch(err => {
console.error(err);
});
```
**[⬆ back to top](#table-of-contents)**
### Async/Await are even cleaner than Promises
Promises are a very clean alternative to callbacks, but ES2017/ES8 brings async and await
which offer an even cleaner solution. All you need is a function that is prefixed
in an `async` keyword, and then you can write your logic imperatively without
a `then` chain of functions. Use this if you can take advantage of ES2017/ES8 features
today!
**Bad:**
```javascript
import { get } from "request-promise";
import { writeFile } from "fs-extra";
get("https://en.wikipedia.org/wiki/Robert_Cecil_Martin")
.then(body => {
return writeFile("article.html", body);
})
.then(() => {
console.log("File written");
})
.catch(err => {
console.error(err);
});
```
**Good:**
```javascript
import { get } from "request-promise";
import { writeFile } from "fs-extra";
async function getCleanCodeArticle() {
try {
const body = await get(
"https://en.wikipedia.org/wiki/Robert_Cecil_Martin"
);
await writeFile("article.html", body);
console.log("File written");
} catch (err) {
console.error(err);
}
}
getCleanCodeArticle()
```
**[⬆ back to top](#table-of-contents)**
## **Error Handling**
Thrown errors are a good thing! They mean the runtime has successfully
identified when something in your program has gone wrong and it's letting
you know by stopping function execution on the current stack, killing the
process (in Node), and notifying you in the console with a stack trace.
### Don't ignore caught errors
Doing nothing with a caught error doesn't give you the ability to ever fix
or react to said error. Logging the error to the console (`console.log`)
isn't much better as often times it can get lost in a sea of things printed
to the console. If you wrap any bit of code in a `try/catch` it means you
think an error may occur there and therefore you should have a plan,
or create a code path, for when it occurs.
**Bad:**
```javascript
try {
functionThatMightThrow();
} catch (error) {
console.log(error);
}
```
**Good:**
```javascript
try {
functionThatMightThrow();
} catch (error) {
// One option (more noisy than console.log):
console.error(error);
// Another option:
notifyUserOfError(error);
// Another option:
reportErrorToService(error);
// OR do all three!
}
```
### Don't ignore rejected promises
For the same reason you shouldn't ignore caught errors
from `try/catch`.
**Bad:**
```javascript
getdata()
.then(data => {
functionThatMightThrow(data);
})
.catch(error => {
console.log(error);
});
```
**Good:**
```javascript
getdata()
.then(data => {
functionThatMightThrow(data);
})
.catch(error => {
// One option (more noisy than console.log):
console.error(error);
// Another option:
notifyUserOfError(error);
// Another option:
reportErrorToService(error);
// OR do all three!
});
```
**[⬆ back to top](#table-of-contents)**
## **Formatting**
Formatting is subjective. Like many rules herein, there is no hard and fast
rule that you must follow. The main point is DO NOT ARGUE over formatting.
There are [tons of tools](https://standardjs.com/rules.html) to automate this.
Use one! It's a waste of time and money for engineers to argue over formatting.
For things that don't fall under the purview of automatic formatting
(indentation, tabs vs. spaces, double vs. single quotes, etc.) look here
for some guidance.
### Use consistent capitalization
JavaScript is untyped, so capitalization tells you a lot about your variables,
functions, etc. These rules are subjective, so your team can choose whatever
they want. The point is, no matter what you all choose, just be consistent.
**Bad:**
```javascript
const DAYS_IN_WEEK = 7;
const daysInMonth = 30;
const songs = ["Back In Black", "Stairway to Heaven", "Hey Jude"];
const Artists = ["ACDC", "Led Zeppelin", "The Beatles"];
function eraseDatabase() {}
function restore_database() {}
class animal {}
class Alpaca {}
```
**Good:**
```javascript
const DAYS_IN_WEEK = 7;
const DAYS_IN_MONTH = 30;
const SONGS = ["Back In Black", "Stairway to Heaven", "Hey Jude"];
const ARTISTS = ["ACDC", "Led Zeppelin", "The Beatles"];
function eraseDatabase() {}
function restoreDatabase() {}
class Animal {}
class Alpaca {}
```
**[⬆ back to top](#table-of-contents)**
### Function callers and callees should be close
If a function calls another, keep those functions vertically close in the source
file. Ideally, keep the caller right above the callee. We tend to read code from
top-to-bottom, like a newspaper. Because of this, make your code read that way.
**Bad:**
```javascript
class PerformanceReview {
constructor(employee) {
this.employee = employee;
}
lookupPeers() {
return db.lookup(this.employee, "peers");
}
lookupManager() {
return db.lookup(this.employee, "manager");
}
getPeerReviews() {
const peers = this.lookupPeers();
// ...
}
perfReview() {
this.getPeerReviews();
this.getManagerReview();
this.getSelfReview();
}
getManagerReview() {
const manager = this.lookupManager();
}
getSelfReview() {
// ...
}
}
const review = new PerformanceReview(employee);
review.perfReview();
```
**Good:**
```javascript
class PerformanceReview {
constructor(employee) {
this.employee = employee;
}
perfReview() {
this.getPeerReviews();
this.getManagerReview();
this.getSelfReview();
}
getPeerReviews() {
const peers = this.lookupPeers();
// ...
}
lookupPeers() {
return db.lookup(this.employee, "peers");
}
getManagerReview() {
const manager = this.lookupManager();
}
lookupManager() {
return db.lookup(this.employee, "manager");
}
getSelfReview() {
// ...
}
}
const review = new PerformanceReview(employee);
review.perfReview();
```
**[⬆ back to top](#table-of-contents)**
## **Comments**
### Only comment things that have business logic complexity.
Comments are an apology, not a requirement. Good code _mostly_ documents itself.
**Bad:**
```javascript
function hashIt(data) {
// The hash
let hash = 0;
// Length of string
const length = data.length;
// Loop through every character in data
for (let i = 0; i < length; i++) {
// Get character code.
const char = data.charCodeAt(i);
// Make the hash
hash = (hash << 5) - hash + char;
// Convert to 32-bit integer
hash &= hash;
}
}
```
**Good:**
```javascript
function hashIt(data) {
let hash = 0;
const length = data.length;
for (let i = 0; i < length; i++) {
const char = data.charCodeAt(i);
hash = (hash << 5) - hash + char;
// Convert to 32-bit integer
hash &= hash;
}
}
```
**[⬆ back to top](#table-of-contents)**
### Don't leave commented out code in your codebase
Version control exists for a reason. Leave old code in your history.
**Bad:**
```javascript
doStuff();
// doOtherStuff();
// doSomeMoreStuff();
// doSoMuchStuff();
```
**Good:**
```javascript
doStuff();
```
**[⬆ back to top](#table-of-contents)**
### Don't have journal comments
Remember, use version control! There's no need for dead code, commented code,
and especially journal comments. Use `git log` to get history!
**Bad:**
```javascript
/**
* 2016-12-20: Removed monads, didn't understand them (RM)
* 2016-10-01: Improved using special monads (JP)
* 2016-02-03: Removed type-checking (LI)
* 2015-03-14: Added combine with type-checking (JR)
*/
function combine(a, b) {
return a + b;
}
```
**Good:**
```javascript
function combine(a, b) {
return a + b;
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid positional markers
They usually just add noise. Let the functions and variable names along with the
proper indentation and formatting give the visual structure to your code.
**Bad:**
```javascript
////////////////////////////////////////////////////////////////////////////////
// Scope Model Instantiation
////////////////////////////////////////////////////////////////////////////////
$scope.model = {
menu: "foo",
nav: "bar"
};
////////////////////////////////////////////////////////////////////////////////
// Action setup
////////////////////////////////////////////////////////////////////////////////
const actions = function() {
// ...
};
```
**Good:**
```javascript
$scope.model = {
menu: "foo",
nav: "bar"
};
const actions = function() {
// ...
};
```
**[⬆ back to top](#table-of-contents)**
## Translation
This is also available in other languages:
- ![am](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Armenia.png) **Armenian**: [hanumanum/clean-code-javascript/](https://github.com/hanumanum/clean-code-javascript)
- ![bd](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Bangladesh.png) **Bangla(বাংলা)**: [InsomniacSabbir/clean-code-javascript/](https://github.com/InsomniacSabbir/clean-code-javascript/)
- ![br](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Brazil.png) **Brazilian Portuguese**: [fesnt/clean-code-javascript](https://github.com/fesnt/clean-code-javascript)
- ![cn](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/China.png) **Simplified Chinese**:
- [alivebao/clean-code-js](https://github.com/alivebao/clean-code-js)
- [beginor/clean-code-javascript](https://github.com/beginor/clean-code-javascript)
- ![tw](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Taiwan.png) **Traditional Chinese**: [AllJointTW/clean-code-javascript](https://github.com/AllJointTW/clean-code-javascript)
- ![fr](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/France.png) **French**: [eugene-augier/clean-code-javascript-fr](https://github.com/eugene-augier/clean-code-javascript-fr)
- ![de](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Germany.png) **German**: [marcbruederlin/clean-code-javascript](https://github.com/marcbruederlin/clean-code-javascript)
- ![id](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Indonesia.png) **Indonesia**: [andirkh/clean-code-javascript/](https://github.com/andirkh/clean-code-javascript/)
- ![it](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Italy.png) **Italian**: [frappacchio/clean-code-javascript/](https://github.com/frappacchio/clean-code-javascript/)
- ![ja](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Japan.png) **Japanese**: [mitsuruog/clean-code-javascript/](https://github.com/mitsuruog/clean-code-javascript/)
- ![kr](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/South-Korea.png) **Korean**: [qkraudghgh/clean-code-javascript-ko](https://github.com/qkraudghgh/clean-code-javascript-ko)
- ![pl](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Poland.png) **Polish**: [greg-dev/clean-code-javascript-pl](https://github.com/greg-dev/clean-code-javascript-pl)
- ![ru](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Russia.png) **Russian**:
- [BoryaMogila/clean-code-javascript-ru/](https://github.com/BoryaMogila/clean-code-javascript-ru/)
- [maksugr/clean-code-javascript](https://github.com/maksugr/clean-code-javascript)
- ![es](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Spain.png) **Spanish**: [tureey/clean-code-javascript](https://github.com/tureey/clean-code-javascript)
- ![es](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Uruguay.png) **Spanish**: [andersontr15/clean-code-javascript](https://github.com/andersontr15/clean-code-javascript-es)
- ![rs](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Serbia.png) **Serbian**: [doskovicmilos/clean-code-javascript/](https://github.com/doskovicmilos/clean-code-javascript)
- ![tr](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Turkey.png) **Turkish**: [bsonmez/clean-code-javascript](https://github.com/bsonmez/clean-code-javascript/tree/turkish-translation)
- ![ua](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Ukraine.png) **Ukrainian**: [mindfr1k/clean-code-javascript-ua](https://github.com/mindfr1k/clean-code-javascript-ua)
- ![vi](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Vietnam.png) **Vietnamese**: [hienvd/clean-code-javascript/](https://github.com/hienvd/clean-code-javascript/)
- ![ir](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Iran.png) **Persian**: [hamettio/clean-code-javascript](https://github.com/hamettio/clean-code-javascript)
**[⬆ back to top](#table-of-contents)**
# clean-code-typescript [![Tweet](https://img.shields.io/twitter/url/http/shields.io.svg?style=social)](https://twitter.com/intent/tweet?text=Clean%20Code%20Typescript&url=https://github.com/labs42io/clean-code-typescript)
Clean Code concepts adapted for TypeScript.
Inspired from [clean-code-javascript](https://github.com/ryanmcdermott/clean-code-javascript).
## Table of Contents
1. [Introduction](#introduction)
2. [Variables](#variables)
3. [Functions](#functions)
4. [Objects and Data Structures](#objects-and-data-structures)
5. [Classes](#classes)
6. [SOLID](#solid)
7. [Testing](#testing)
8. [Concurrency](#concurrency)
9. [Error Handling](#error-handling)
10. [Formatting](#formatting)
11. [Comments](#comments)
12. [Translations](#translations)
## Introduction
![Humorous image of software quality estimation as a count of how many expletives
you shout when reading code](https://www.osnews.com/images/comics/wtfm.jpg)
Software engineering principles, from Robert C. Martin's book
[*Clean Code*](https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882),
adapted for TypeScript. This is not a style guide. It's a guide to producing
[readable, reusable, and refactorable](https://github.com/ryanmcdermott/3rs-of-software-architecture) software in TypeScript.
Not every principle herein has to be strictly followed, and even fewer will be
universally agreed upon. These are guidelines and nothing more, but they are
ones codified over many years of collective experience by the authors of
*Clean Code*.
Our craft of software engineering is just a bit over 50 years old, and we are
still learning a lot. When software architecture is as old as architecture
itself, maybe then we will have harder rules to follow. For now, let these
guidelines serve as a touchstone by which to assess the quality of the
TypeScript code that you and your team produce.
One more thing: knowing these won't immediately make you a better software
developer, and working with them for many years doesn't mean you won't make
mistakes. Every piece of code starts as a first draft, like wet clay getting
shaped into its final form. Finally, we chisel away the imperfections when
we review it with our peers. Don't beat yourself up for first drafts that need
improvement. Beat up the code instead!
**[⬆ back to top](#table-of-contents)**
## Variables
### Use meaningful variable names
Distinguish names in such a way that the reader knows what the differences offer.
**Bad:**
```ts
function between<T>(a1: T, a2: T, a3: T): boolean {
return a2 <= a1 && a1 <= a3;
}
```
**Good:**
```ts
function between<T>(value: T, left: T, right: T): boolean {
return left <= value && value <= right;
}
```
**[⬆ back to top](#table-of-contents)**
### Use pronounceable variable names
If you can’t pronounce it, you can’t discuss it without sounding like an idiot.
**Bad:**
```ts
type DtaRcrd102 = {
genymdhms: Date;
modymdhms: Date;
pszqint: number;
}
```
**Good:**
```ts
type Customer = {
generationTimestamp: Date;
modificationTimestamp: Date;
recordId: number;
}
```
**[⬆ back to top](#table-of-contents)**
### Use the same vocabulary for the same type of variable
**Bad:**
```ts
function getUserInfo(): User;
function getUserDetails(): User;
function getUserData(): User;
```
**Good:**
```ts
function getUser(): User;
```
**[⬆ back to top](#table-of-contents)**
### Use searchable names
We will read more code than we will ever write. It's important that the code we do write must be readable and searchable. By *not* naming variables that end up being meaningful for understanding our program, we hurt our readers. Make your names searchable. Tools like [ESLint](https://typescript-eslint.io/) can help identify unnamed constants (also known as magic strings and magic numbers).
**Bad:**
```ts
// What the heck is 86400000 for?
setTimeout(restart, 86400000);
```
**Good:**
```ts
// Declare them as capitalized named constants.
const MILLISECONDS_PER_DAY = 24 * 60 * 60 * 1000; // 86400000
setTimeout(restart, MILLISECONDS_PER_DAY);
```
**[⬆ back to top](#table-of-contents)**
### Use explanatory variables
**Bad:**
```ts
declare const users: Map<string, User>;
for (const keyValue of users) {
// iterate through users map
}
```
**Good:**
```ts
declare const users: Map<string, User>;
for (const [id, user] of users) {
// iterate through users map
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid Mental Mapping
Explicit is better than implicit.
*Clarity is king.*
**Bad:**
```ts
const u = getUser();
const s = getSubscription();
const t = charge(u, s);
```
**Good:**
```ts
const user = getUser();
const subscription = getSubscription();
const transaction = charge(user, subscription);
```
**[⬆ back to top](#table-of-contents)**
### Don't add unneeded context
If your class/type/object name tells you something, don't repeat that in your variable name.
**Bad:**
```ts
type Car = {
carMake: string;
carModel: string;
carColor: string;
}
function print(car: Car): void {
console.log(`${car.carMake} ${car.carModel} (${car.carColor})`);
}
```
**Good:**
```ts
type Car = {
make: string;
model: string;
color: string;
}
function print(car: Car): void {
console.log(`${car.make} ${car.model} (${car.color})`);
}
```
**[⬆ back to top](#table-of-contents)**
### Use default arguments instead of short circuiting or conditionals
Default arguments are often cleaner than short circuiting.
**Bad:**
```ts
function loadPages(count?: number) {
const loadCount = count !== undefined ? count : 10;
// ...
}
```
**Good:**
```ts
function loadPages(count: number = 10) {
// ...
}
```
**[⬆ back to top](#table-of-contents)**
### Use enum to document the intent
Enums can help you document the intent of the code. For example when we are concerned about values being
different rather than the exact value of those.
**Bad:**
```ts
const GENRE = {
ROMANTIC: 'romantic',
DRAMA: 'drama',
COMEDY: 'comedy',
DOCUMENTARY: 'documentary',
}
projector.configureFilm(GENRE.COMEDY);
class Projector {
// declaration of Projector
configureFilm(genre) {
switch (genre) {
case GENRE.ROMANTIC:
// some logic to be executed
}
}
}
```
**Good:**
```ts
enum GENRE {
ROMANTIC,
DRAMA,
COMEDY,
DOCUMENTARY,
}
projector.configureFilm(GENRE.COMEDY);
class Projector {
// declaration of Projector
configureFilm(genre) {
switch (genre) {
case GENRE.ROMANTIC:
// some logic to be executed
}
}
}
```
**[⬆ back to top](#table-of-contents)**
## Functions
### Function arguments (2 or fewer ideally)
Limiting the number of function parameters is incredibly important because it makes testing your function easier.
Having more than three leads to a combinatorial explosion where you have to test tons of different cases with each separate argument.
One or two arguments is the ideal case, and three should be avoided if possible. Anything more than that should be consolidated.
Usually, if you have more than two arguments then your function is trying to do too much.
In cases where it's not, most of the time a higher-level object will suffice as an argument.
Consider using object literals if you are finding yourself needing a lot of arguments.
To make it obvious what properties the function expects, you can use the [destructuring](https://basarat.gitbook.io/typescript/future-javascript/destructuring) syntax.
This has a few advantages:
1. When someone looks at the function signature, it's immediately clear what properties are being used.
2. It can be used to simulate named parameters.
3. Destructuring also clones the specified primitive values of the argument object passed into the function. This can help prevent side effects. Note: objects and arrays that are destructured from the argument object are NOT cloned.
4. TypeScript warns you about unused properties, which would be impossible without destructuring.
**Bad:**
```ts
function createMenu(title: string, body: string, buttonText: string, cancellable: boolean) {
// ...
}
createMenu('Foo', 'Bar', 'Baz', true);
```
**Good:**
```ts
function createMenu(options: { title: string, body: string, buttonText: string, cancellable: boolean }) {
// ...
}
createMenu({
title: 'Foo',
body: 'Bar',
buttonText: 'Baz',
cancellable: true
});
```
You can further improve readability by using [type aliases](https://www.typescriptlang.org/docs/handbook/advanced-types.html#type-aliases):
```ts
type MenuOptions = { title: string, body: string, buttonText: string, cancellable: boolean };
function createMenu(options: MenuOptions) {
// ...
}
createMenu({
title: 'Foo',
body: 'Bar',
buttonText: 'Baz',
cancellable: true
});
```
**[⬆ back to top](#table-of-contents)**
### Functions should do one thing
This is by far the most important rule in software engineering. When functions do more than one thing, they are harder to compose, test, and reason about. When you can isolate a function to just one action, it can be refactored easily and your code will read much cleaner. If you take nothing else away from this guide other than this, you'll be ahead of many developers.
**Bad:**
```ts
function emailActiveClients(clients: Client[]) {
clients.forEach((client) => {
const clientRecord = database.lookup(client);
if (clientRecord.isActive()) {
email(client);
}
});
}
```
**Good:**
```ts
function emailActiveClients(clients: Client[]) {
clients.filter(isActiveClient).forEach(email);
}
function isActiveClient(client: Client) {
const clientRecord = database.lookup(client);
return clientRecord.isActive();
}
```
**[⬆ back to top](#table-of-contents)**
### Function names should say what they do
**Bad:**
```ts
function addToDate(date: Date, month: number): Date {
// ...
}
const date = new Date();
// It's hard to tell from the function name what is added
addToDate(date, 1);
```
**Good:**
```ts
function addMonthToDate(date: Date, month: number): Date {
// ...
}
const date = new Date();
addMonthToDate(date, 1);
```
**[⬆ back to top](#table-of-contents)**
### Functions should only be one level of abstraction
When you have more than one level of abstraction your function is usually doing too much. Splitting up functions leads to reusability and easier testing.
**Bad:**
```ts
function parseCode(code: string) {
const REGEXES = [ /* ... */ ];
const statements = code.split(' ');
const tokens = [];
REGEXES.forEach((regex) => {
statements.forEach((statement) => {
// ...
});
});
const ast = [];
tokens.forEach((token) => {
// lex...
});
ast.forEach((node) => {
// parse...
});
}
```
**Good:**
```ts
const REGEXES = [ /* ... */ ];
function parseCode(code: string) {
const tokens = tokenize(code);
const syntaxTree = parse(tokens);
syntaxTree.forEach((node) => {
// parse...
});
}
function tokenize(code: string): Token[] {
const statements = code.split(' ');
const tokens: Token[] = [];
REGEXES.forEach((regex) => {
statements.forEach((statement) => {
tokens.push( /* ... */ );
});
});
return tokens;
}
function parse(tokens: Token[]): SyntaxTree {
const syntaxTree: SyntaxTree[] = [];
tokens.forEach((token) => {
syntaxTree.push( /* ... */ );
});
return syntaxTree;
}
```
**[⬆ back to top](#table-of-contents)**
### Remove duplicate code
Do your absolute best to avoid duplicate code.
Duplicate code is bad because it means that there's more than one place to alter something if you need to change some logic.
Imagine if you run a restaurant and you keep track of your inventory: all your tomatoes, onions, garlic, spices, etc.
If you have multiple lists that you keep this on, then all have to be updated when you serve a dish with tomatoes in them.
If you only have one list, there's only one place to update!
Oftentimes you have duplicate code because you have two or more slightly different things, that share a lot in common, but their differences force you to have two or more separate functions that do much of the same things. Removing duplicate code means creating an abstraction that can handle this set of different things with just one function/module/class.
Getting the abstraction right is critical, that's why you should follow the [SOLID](#solid) principles. Bad abstractions can be worse than duplicate code, so be careful! Having said this, if you can make a good abstraction, do it! Don't repeat yourself, otherwise, you'll find yourself updating multiple places anytime you want to change one thing.
**Bad:**
```ts
function showDeveloperList(developers: Developer[]) {
developers.forEach((developer) => {
const expectedSalary = developer.calculateExpectedSalary();
const experience = developer.getExperience();
const githubLink = developer.getGithubLink();
const data = {
expectedSalary,
experience,
githubLink
};
render(data);
});
}
function showManagerList(managers: Manager[]) {
managers.forEach((manager) => {
const expectedSalary = manager.calculateExpectedSalary();
const experience = manager.getExperience();
const portfolio = manager.getMBAProjects();
const data = {
expectedSalary,
experience,
portfolio
};
render(data);
});
}
```
**Good:**
```ts
class Developer {
// ...
getExtraDetails() {
return {
githubLink: this.githubLink,
}
}
}
class Manager {
// ...
getExtraDetails() {
return {
portfolio: this.portfolio,
}
}
}
function showEmployeeList(employee: (Developer | Manager)[]) {
employee.forEach((employee) => {
const expectedSalary = employee.calculateExpectedSalary();
const experience = employee.getExperience();
const extra = employee.getExtraDetails();
const data = {
expectedSalary,
experience,
extra,
};
render(data);
});
}
```
You may also consider adding a union type, or common parent class if it suits your abstraction.
```ts
class Developer {
// ...
}
class Manager {
// ...
}
type Employee = Developer | Manager
function showEmployeeList(employee: Employee[]) {
// ...
});
}
```
You should be critical about code duplication. Sometimes there is a tradeoff between duplicated code and increased complexity by introducing unnecessary abstraction. When two implementations from two different modules look similar but live in different domains, duplication might be acceptable and preferred over extracting the common code. The extracted common code, in this case, introduces an indirect dependency between the two modules.
**[⬆ back to top](#table-of-contents)**
### Set default objects with Object.assign or destructuring
**Bad:**
```ts
type MenuConfig = { title?: string, body?: string, buttonText?: string, cancellable?: boolean };
function createMenu(config: MenuConfig) {
config.title = config.title || 'Foo';
config.body = config.body || 'Bar';
config.buttonText = config.buttonText || 'Baz';
config.cancellable = config.cancellable !== undefined ? config.cancellable : true;
// ...
}
createMenu({ body: 'Bar' });
```
**Good:**
```ts
type MenuConfig = { title?: string, body?: string, buttonText?: string, cancellable?: boolean };
function createMenu(config: MenuConfig) {
const menuConfig = Object.assign({
title: 'Foo',
body: 'Bar',
buttonText: 'Baz',
cancellable: true
}, config);
// ...
}
createMenu({ body: 'Bar' });
```
Or, you could use the spread operator:
```ts
function createMenu(config: MenuConfig) {
const menuConfig = {
title: 'Foo',
body: 'Bar',
buttonText: 'Baz',
cancellable: true,
...config,
};
// ...
}
```
The spread operator and `Object.assign()` are very similar.
The main difference is that spreading defines new properties, while `Object.assign()` sets them. More detailed, the difference is explained in [this](https://stackoverflow.com/questions/32925460/object-spread-vs-object-assign) thread.
Alternatively, you can use destructuring with default values:
```ts
type MenuConfig = { title?: string, body?: string, buttonText?: string, cancellable?: boolean };
function createMenu({ title = 'Foo', body = 'Bar', buttonText = 'Baz', cancellable = true }: MenuConfig) {
// ...
}
createMenu({ body: 'Bar' });
```
To avoid any side effects and unexpected behavior by passing in explicitly the `undefined` or `null` value, you can tell the TypeScript compiler to not allow it.
See [`--strictNullChecks`](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0.html#--strictnullchecks) option in TypeScript.
**[⬆ back to top](#table-of-contents)**
### Don't use flags as function parameters
Flags tell your user that this function does more than one thing.
Functions should do one thing. Split out your functions if they are following different code paths based on a boolean.
**Bad:**
```ts
function createFile(name: string, temp: boolean) {
if (temp) {
fs.create(`./temp/${name}`);
} else {
fs.create(name);
}
}
```
**Good:**
```ts
function createTempFile(name: string) {
createFile(`./temp/${name}`);
}
function createFile(name: string) {
fs.create(name);
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid Side Effects (part 1)
A function produces a side effect if it does anything other than take a value in and return another value or values.
A side effect could be writing to a file, modifying some global variable, or accidentally wiring all your money to a stranger.
Now, you do need to have side effects in a program on occasion. Like the previous example, you might need to write to a file.
What you want to do is to centralize where you are doing this. Don't have several functions and classes that write to a particular file.
Have one service that does it. One and only one.
The main point is to avoid common pitfalls like sharing state between objects without any structure, using mutable data types that can be written to by anything, and not centralizing where your side effects occur. If you can do this, you will be happier than the vast majority of other programmers.
**Bad:**
```ts
// Global variable referenced by following function.
let name = 'Robert C. Martin';
function toBase64() {
name = btoa(name);
}
toBase64();
// If we had another function that used this name, now it'd be a Base64 value
console.log(name); // expected to print 'Robert C. Martin' but instead 'Um9iZXJ0IEMuIE1hcnRpbg=='
```
**Good:**
```ts
const name = 'Robert C. Martin';
function toBase64(text: string): string {
return btoa(text);
}
const encodedName = toBase64(name);
console.log(name);
```
**[⬆ back to top](#table-of-contents)**
### Avoid Side Effects (part 2)
Browsers and Node.js process only JavaScript, therefore any TypeScript code has to be compiled before running or debugging. In JavaScript, some values are unchangeable (immutable) and some are changeable (mutable). Objects and arrays are two kinds of mutable values so it's important to handle them carefully when they're passed as parameters to a function. A JavaScript function can change an object's properties or alter the contents of an array which could easily cause bugs elsewhere.
Suppose there's a function that accepts an array parameter representing a shopping cart. If the function makes a change in that shopping cart array - by adding an item to purchase, for example - then any other function that uses that same `cart` array will be affected by this addition. That may be great, however it could also be bad. Let's imagine a bad situation:
The user clicks the "Purchase" button which calls a `purchase` function that spawns a network request and sends the `cart` array to the server. Because of a bad network connection, the `purchase` function has to keep retrying the request. Now, what if in the meantime the user accidentally clicks an "Add to Cart" button on an item they don't actually want before the network request begins? If that happens and the network request begins, then that purchase function will send the accidentally added item because the `cart` array was modified.
A great solution would be for the `addItemToCart` function to always clone the `cart`, edit it, and return the clone. This would ensure that functions that are still using the old shopping cart wouldn't be affected by the changes.
Two caveats to mention to this approach:
1. There might be cases where you actually want to modify the input object, but when you adopt this programming practice you will find that those cases are pretty rare. Most things can be refactored to have no side effects! (see [pure function](https://en.wikipedia.org/wiki/Pure_function))
2. Cloning big objects can be very expensive in terms of performance. Luckily, this isn't a big issue in practice because there are [great libraries](https://github.com/immutable-js/immutable-js) that allow this kind of programming approach to be fast and not as memory intensive as it would be for you to manually clone objects and arrays.
**Bad:**
```ts
function addItemToCart(cart: CartItem[], item: Item): void {
cart.push({ item, date: Date.now() });
};
```
**Good:**
```ts
function addItemToCart(cart: CartItem[], item: Item): CartItem[] {
return [...cart, { item, date: Date.now() }];
};
```
**[⬆ back to top](#table-of-contents)**
### Don't write to global functions
Polluting globals is a bad practice in JavaScript because you could clash with another library and the user of your API would be none-the-wiser until they get an exception in production. Let's think about an example: what if you wanted to extend JavaScript's native Array method to have a `diff` method that could show the difference between two arrays? You could write your new function to the `Array.prototype`, but it could clash with another library that tried to do the same thing. What if that other library was just using `diff` to find the difference between the first and last elements of an array? This is why it would be much better to just use classes and simply extend the `Array` global.
**Bad:**
```ts
declare global {
interface Array<T> {
diff(other: T[]): Array<T>;
}
}
if (!Array.prototype.diff) {
Array.prototype.diff = function <T>(other: T[]): T[] {
const hash = new Set(other);
return this.filter(elem => !hash.has(elem));
};
}
```
**Good:**
```ts
class MyArray<T> extends Array<T> {
diff(other: T[]): T[] {
const hash = new Set(other);
return this.filter(elem => !hash.has(elem));
};
}
```
**[⬆ back to top](#table-of-contents)**
### Favor functional programming over imperative programming
Favor this style of programming when you can.
**Bad:**
```ts
const contributions = [
{
name: 'Uncle Bobby',
linesOfCode: 500
}, {
name: 'Suzie Q',
linesOfCode: 1500
}, {
name: 'Jimmy Gosling',
linesOfCode: 150
}, {
name: 'Gracie Hopper',
linesOfCode: 1000
}
];
let totalOutput = 0;
for (let i = 0; i < contributions.length; i++) {
totalOutput += contributions[i].linesOfCode;
}
```
**Good:**
```ts
const contributions = [
{
name: 'Uncle Bobby',
linesOfCode: 500
}, {
name: 'Suzie Q',
linesOfCode: 1500
}, {
name: 'Jimmy Gosling',
linesOfCode: 150
}, {
name: 'Gracie Hopper',
linesOfCode: 1000
}
];
const totalOutput = contributions
.reduce((totalLines, output) => totalLines + output.linesOfCode, 0);
```
**[⬆ back to top](#table-of-contents)**
### Encapsulate conditionals
**Bad:**
```ts
if (subscription.isTrial || account.balance > 0) {
// ...
}
```
**Good:**
```ts
function canActivateService(subscription: Subscription, account: Account) {
return subscription.isTrial || account.balance > 0;
}
if (canActivateService(subscription, account)) {
// ...
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid negative conditionals
**Bad:**
```ts
function isEmailNotUsed(email: string): boolean {
// ...
}
if (isEmailNotUsed(email)) {
// ...
}
```
**Good:**
```ts
function isEmailUsed(email: string): boolean {
// ...
}
if (!isEmailUsed(email)) {
// ...
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid conditionals
This seems like an impossible task. Upon first hearing this, most people say, "how am I supposed to do anything without an `if` statement?" The answer is that you can use polymorphism to achieve the same task in many cases. The second question is usually, "well that's great but why would I want to do that?" The answer is a previous clean code concept we learned: a function should only do one thing. When you have classes and functions that have `if` statements, you are telling your user that your function does more than one thing. Remember, just do one thing.
**Bad:**
```ts
class Airplane {
private type: string;
// ...
getCruisingAltitude() {
switch (this.type) {
case '777':
return this.getMaxAltitude() - this.getPassengerCount();
case 'Air Force One':
return this.getMaxAltitude();
case 'Cessna':
return this.getMaxAltitude() - this.getFuelExpenditure();
default:
throw new Error('Unknown airplane type.');
}
}
private getMaxAltitude(): number {
// ...
}
}
```
**Good:**
```ts
abstract class Airplane {
protected getMaxAltitude(): number {
// shared logic with subclasses ...
}
// ...
}
class Boeing777 extends Airplane {
// ...
getCruisingAltitude() {
return this.getMaxAltitude() - this.getPassengerCount();
}
}
class AirForceOne extends Airplane {
// ...
getCruisingAltitude() {
return this.getMaxAltitude();
}
}
class Cessna extends Airplane {
// ...
getCruisingAltitude() {
return this.getMaxAltitude() - this.getFuelExpenditure();
}
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid type checking
TypeScript is a strict syntactical superset of JavaScript and adds optional static type checking to the language.
Always prefer to specify types of variables, parameters and return values to leverage the full power of TypeScript features.
It makes refactoring more easier.
**Bad:**
```ts
function travelToTexas(vehicle: Bicycle | Car) {
if (vehicle instanceof Bicycle) {
vehicle.pedal(currentLocation, new Location('texas'));
} else if (vehicle instanceof Car) {
vehicle.drive(currentLocation, new Location('texas'));
}
}
```
**Good:**
```ts
type Vehicle = Bicycle | Car;
function travelToTexas(vehicle: Vehicle) {
vehicle.move(currentLocation, new Location('texas'));
}
```
**[⬆ back to top](#table-of-contents)**
### Don't over-optimize
Modern browsers do a lot of optimization under-the-hood at runtime. A lot of times, if you are optimizing then you are just wasting your time. There are good [resources](https://github.com/petkaantonov/bluebird/wiki/Optimization-killers) for seeing where optimization is lacking. Target those in the meantime, until they are fixed if they can be.
**Bad:**
```ts
// On old browsers, each iteration with uncached `list.length` would be costly
// because of `list.length` recomputation. In modern browsers, this is optimized.
for (let i = 0, len = list.length; i < len; i++) {
// ...
}
```
**Good:**
```ts
for (let i = 0; i < list.length; i++) {
// ...
}
```
**[⬆ back to top](#table-of-contents)**
### Remove dead code
Dead code is just as bad as duplicate code. There's no reason to keep it in your codebase.
If it's not being called, get rid of it! It will still be saved in your version history if you still need it.
**Bad:**
```ts
function oldRequestModule(url: string) {
// ...
}
function requestModule(url: string) {
// ...
}
const req = requestModule;
inventoryTracker('apples', req, 'www.inventory-awesome.io');
```
**Good:**
```ts
function requestModule(url: string) {
// ...
}
const req = requestModule;
inventoryTracker('apples', req, 'www.inventory-awesome.io');
```
**[⬆ back to top](#table-of-contents)**
### Use iterators and generators
Use generators and iterables when working with collections of data used like a stream.
There are some good reasons:
- decouples the callee from the generator implementation in a sense that callee decides how many
items to access
- lazy execution, items are streamed on-demand
- built-in support for iterating items using the `for-of` syntax
- iterables allow implementing optimized iterator patterns
**Bad:**
```ts
function fibonacci(n: number): number[] {
if (n === 1) return [0];
if (n === 2) return [0, 1];
const items: number[] = [0, 1];
while (items.length < n) {
items.push(items[items.length - 2] + items[items.length - 1]);
}
return items;
}
function print(n: number) {
fibonacci(n).forEach(fib => console.log(fib));
}
// Print first 10 Fibonacci numbers.
print(10);
```
**Good:**
```ts
// Generates an infinite stream of Fibonacci numbers.
// The generator doesn't keep the array of all numbers.
function* fibonacci(): IterableIterator<number> {
let [a, b] = [0, 1];
while (true) {
yield a;
[a, b] = [b, a + b];
}
}
function print(n: number) {
let i = 0;
for (const fib of fibonacci()) {
if (i++ === n) break;
console.log(fib);
}
}
// Print first 10 Fibonacci numbers.
print(10);
```
There are libraries that allow working with iterables in a similar way as with native arrays, by
chaining methods like `map`, `slice`, `forEach` etc. See [itiriri](https://www.npmjs.com/package/itiriri) for
an example of advanced manipulation with iterables (or [itiriri-async](https://www.npmjs.com/package/itiriri-async) for manipulation of async iterables).
```ts
import itiriri from 'itiriri';
function* fibonacci(): IterableIterator<number> {
let [a, b] = [0, 1];
while (true) {
yield a;
[a, b] = [b, a + b];
}
}
itiriri(fibonacci())
.take(10)
.forEach(fib => console.log(fib));
```
**[⬆ back to top](#table-of-contents)**
## Objects and Data Structures
### Use getters and setters
TypeScript supports getter/setter syntax.
Using getters and setters to access data from objects that encapsulate behavior could be better than simply looking for a property on an object.
"Why?" you might ask. Well, here's a list of reasons:
- When you want to do more beyond getting an object property, you don't have to look up and change every accessor in your codebase.
- Makes adding validation simple when doing a `set`.
- Encapsulates the internal representation.
- Easy to add logging and error handling when getting and setting.
- You can lazy load your object's properties, let's say getting it from a server.
**Bad:**
```ts
type BankAccount = {
balance: number;
// ...
}
const value = 100;
const account: BankAccount = {
balance: 0,
// ...
};
if (value < 0) {
throw new Error('Cannot set negative balance.');
}
account.balance = value;
```
**Good:**
```ts
class BankAccount {
private accountBalance: number = 0;
get balance(): number {
return this.accountBalance;
}
set balance(value: number) {
if (value < 0) {
throw new Error('Cannot set negative balance.');
}
this.accountBalance = value;
}
// ...
}
// Now `BankAccount` encapsulates the validation logic.
// If one day the specifications change, and we need extra validation rule,
// we would have to alter only the `setter` implementation,
// leaving all dependent code unchanged.
const account = new BankAccount();
account.balance = 100;
```
**[⬆ back to top](#table-of-contents)**
### Make objects have private/protected members
TypeScript supports `public` *(default)*, `protected` and `private` accessors on class members.
**Bad:**
```ts
class Circle {
radius: number;
constructor(radius: number) {
this.radius = radius;
}
perimeter() {
return 2 * Math.PI * this.radius;
}
surface() {
return Math.PI * this.radius * this.radius;
}
}
```
**Good:**
```ts
class Circle {
constructor(private readonly radius: number) {
}
perimeter() {
return 2 * Math.PI * this.radius;
}
surface() {
return Math.PI * this.radius * this.radius;
}
}
```
**[⬆ back to top](#table-of-contents)**
### Prefer immutability
TypeScript's type system allows you to mark individual properties on an interface/class as *readonly*. This allows you to work in a functional way (an unexpected mutation is bad).
For more advanced scenarios there is a built-in type `Readonly` that takes a type `T` and marks all of its properties as readonly using mapped types (see [mapped types](https://www.typescriptlang.org/docs/handbook/advanced-types.html#mapped-types)).
**Bad:**
```ts
interface Config {
host: string;
port: string;
db: string;
}
```
**Good:**
```ts
interface Config {
readonly host: string;
readonly port: string;
readonly db: string;
}
```
For arrays, you can create a read-only array by using `ReadonlyArray<T>`.
It doesn't allow changes such as `push()` and `fill()`, but can use features such as `concat()` and `slice()` that do not change the array's value.
**Bad:**
```ts
const array: number[] = [ 1, 3, 5 ];
array = []; // error
array.push(100); // array will be updated
```
**Good:**
```ts
const array: ReadonlyArray<number> = [ 1, 3, 5 ];
array = []; // error
array.push(100); // error
```
Declaring read-only arguments in [TypeScript 3.4 is a bit easier](https://github.com/microsoft/TypeScript/wiki/What's-new-in-TypeScript#improvements-for-readonlyarray-and-readonly-tuples).
```ts
function hoge(args: readonly string[]) {
args.push(1); // error
}
```
Prefer [const assertions](https://github.com/microsoft/TypeScript/wiki/What's-new-in-TypeScript#const-assertions) for literal values.
**Bad:**
```ts
const config = {
hello: 'world'
};
config.hello = 'world'; // value is changed
const array = [ 1, 3, 5 ];
array[0] = 10; // value is changed
// writable objects is returned
function readonlyData(value: number) {
return { value };
}
const result = readonlyData(100);
result.value = 200; // value is changed
```
**Good:**
```ts
// read-only object
const config = {
hello: 'world'
} as const;
config.hello = 'world'; // error
// read-only array
const array = [ 1, 3, 5 ] as const;
array[0] = 10; // error
// You can return read-only objects
function readonlyData(value: number) {
return { value } as const;
}
const result = readonlyData(100);
result.value = 200; // error
```
**[⬆ back to top](#table-of-contents)**
### type vs. interface
Use type when you might need a union or intersection. Use an interface when you want `extends` or `implements`. There is no strict rule, however, use the one that works for you.
For a more detailed explanation refer to this [answer](https://stackoverflow.com/questions/37233735/typescript-interfaces-vs-types/54101543#54101543) about the differences between `type` and `interface` in TypeScript.
**Bad:**
```ts
interface EmailConfig {
// ...
}
interface DbConfig {
// ...
}
interface Config {
// ...
}
//...
type Shape = {
// ...
}
```
**Good:**
```ts
type EmailConfig = {
// ...
}
type DbConfig = {
// ...
}
type Config = EmailConfig | DbConfig;
// ...
interface Shape {
// ...
}
class Circle implements Shape {
// ...
}
class Square implements Shape {
// ...
}
```
**[⬆ back to top](#table-of-contents)**
## Classes
### Classes should be small
The class' size is measured by its responsibility. Following the *Single Responsibility principle* a class should be small.
**Bad:**
```ts
class Dashboard {
getLanguage(): string { /* ... */ }
setLanguage(language: string): void { /* ... */ }
showProgress(): void { /* ... */ }
hideProgress(): void { /* ... */ }
isDirty(): boolean { /* ... */ }
disable(): void { /* ... */ }
enable(): void { /* ... */ }
addSubscription(subscription: Subscription): void { /* ... */ }
removeSubscription(subscription: Subscription): void { /* ... */ }
addUser(user: User): void { /* ... */ }
removeUser(user: User): void { /* ... */ }
goToHomePage(): void { /* ... */ }
updateProfile(details: UserDetails): void { /* ... */ }
getVersion(): string { /* ... */ }
// ...
}
```
**Good:**
```ts
class Dashboard {
disable(): void { /* ... */ }
enable(): void { /* ... */ }
getVersion(): string { /* ... */ }
}
// split the responsibilities by moving the remaining methods to other classes
// ...
```
**[⬆ back to top](#table-of-contents)**
### High cohesion and low coupling
Cohesion defines the degree to which class members are related to each other. Ideally, all fields within a class should be used by each method.
We then say that the class is *maximally cohesive*. In practice, this, however, is not always possible, nor even advisable. You should however prefer cohesion to be high.
Coupling refers to how related or dependent are two classes toward each other. Classes are said to be low coupled if changes in one of them don't affect the other one.
Good software design has **high cohesion** and **low coupling**.
**Bad:**
```ts
class UserManager {
// Bad: each private variable is used by one or another group of methods.
// It makes clear evidence that the class is holding more than a single responsibility.
// If I need only to create the service to get the transactions for a user,
// I'm still forced to pass and instance of `emailSender`.
constructor(
private readonly db: Database,
private readonly emailSender: EmailSender) {
}
async getUser(id: number): Promise<User> {
return await db.users.findOne({ id });
}
async getTransactions(userId: number): Promise<Transaction[]> {
return await db.transactions.find({ userId });
}
async sendGreeting(): Promise<void> {
await emailSender.send('Welcome!');
}
async sendNotification(text: string): Promise<void> {
await emailSender.send(text);
}
async sendNewsletter(): Promise<void> {
// ...
}
}
```
**Good:**
```ts
class UserService {
constructor(private readonly db: Database) {
}
async getUser(id: number): Promise<User> {
return await this.db.users.findOne({ id });
}
async getTransactions(userId: number): Promise<Transaction[]> {
return await this.db.transactions.find({ userId });
}
}
class UserNotifier {
constructor(private readonly emailSender: EmailSender) {
}
async sendGreeting(): Promise<void> {
await this.emailSender.send('Welcome!');
}
async sendNotification(text: string): Promise<void> {
await this.emailSender.send(text);
}
async sendNewsletter(): Promise<void> {
// ...
}
}
```
**[⬆ back to top](#table-of-contents)**
### Prefer composition over inheritance
As stated famously in [Design Patterns](https://en.wikipedia.org/wiki/Design_Patterns) by the Gang of Four, you should *prefer composition over inheritance* where you can. There are lots of good reasons to use inheritance and lots of good reasons to use composition. The main point for this maxim is that if your mind instinctively goes for inheritance, try to think if composition could model your problem better. In some cases it can.
You might be wondering then, "when should I use inheritance?" It depends on your problem at hand, but this is a decent list of when inheritance makes more sense than composition:
1. Your inheritance represents an "is-a" relationship and not a "has-a" relationship (Human->Animal vs. User->UserDetails).
2. You can reuse code from the base classes (Humans can move like all animals).
3. You want to make global changes to derived classes by changing a base class. (Change the caloric expenditure of all animals when they move).
**Bad:**
```ts
class Employee {
constructor(
private readonly name: string,
private readonly email: string) {
}
// ...
}
// Bad because Employees "have" tax data. EmployeeTaxData is not a type of Employee
class EmployeeTaxData extends Employee {
constructor(
name: string,
email: string,
private readonly ssn: string,
private readonly salary: number) {
super(name, email);
}
// ...
}
```
**Good:**
```ts
class Employee {
private taxData: EmployeeTaxData;
constructor(
private readonly name: string,
private readonly email: string) {
}
setTaxData(ssn: string, salary: number): Employee {
this.taxData = new EmployeeTaxData(ssn, salary);
return this;
}
// ...
}
class EmployeeTaxData {
constructor(
public readonly ssn: string,
public readonly salary: number) {
}
// ...
}
```
**[⬆ back to top](#table-of-contents)**
### Use method chaining
This pattern is very useful and commonly used in many libraries. It allows your code to be expressive, and less verbose. For that reason, use method chaining and take a look at how clean your code will be.
**Bad:**
```ts
class QueryBuilder {
private collection: string;
private pageNumber: number = 1;
private itemsPerPage: number = 100;
private orderByFields: string[] = [];
from(collection: string): void {
this.collection = collection;
}
page(number: number, itemsPerPage: number = 100): void {
this.pageNumber = number;
this.itemsPerPage = itemsPerPage;
}
orderBy(...fields: string[]): void {
this.orderByFields = fields;
}
build(): Query {
// ...
}
}
// ...
const queryBuilder = new QueryBuilder();
queryBuilder.from('users');
queryBuilder.page(1, 100);
queryBuilder.orderBy('firstName', 'lastName');
const query = queryBuilder.build();
```
**Good:**
```ts
class QueryBuilder {
private collection: string;
private pageNumber: number = 1;
private itemsPerPage: number = 100;
private orderByFields: string[] = [];
from(collection: string): this {
this.collection = collection;
return this;
}
page(number: number, itemsPerPage: number = 100): this {
this.pageNumber = number;
this.itemsPerPage = itemsPerPage;
return this;
}
orderBy(...fields: string[]): this {
this.orderByFields = fields;
return this;
}
build(): Query {
// ...
}
}
// ...
const query = new QueryBuilder()
.from('users')
.page(1, 100)
.orderBy('firstName', 'lastName')
.build();
```
**[⬆ back to top](#table-of-contents)**
## SOLID
### Single Responsibility Principle (SRP)
As stated in Clean Code, "There should never be more than one reason for a class to change". It's tempting to jam-pack a class with a lot of functionality, like when you can only take one suitcase on your flight. The issue with this is that your class won't be conceptually cohesive and it will give it many reasons to change. Minimizing the amount of time you need to change a class is important. It's important because if too much functionality is in one class and you modify a piece of it, it can be difficult to understand how that will affect other dependent modules in your codebase.
**Bad:**
```ts
class UserSettings {
constructor(private readonly user: User) {
}
changeSettings(settings: UserSettings) {
if (this.verifyCredentials()) {
// ...
}
}
verifyCredentials() {
// ...
}
}
```
**Good:**
```ts
class UserAuth {
constructor(private readonly user: User) {
}
verifyCredentials() {
// ...
}
}
class UserSettings {
private readonly auth: UserAuth;
constructor(private readonly user: User) {
this.auth = new UserAuth(user);
}
changeSettings(settings: UserSettings) {
if (this.auth.verifyCredentials()) {
// ...
}
}
}
```
**[⬆ back to top](#table-of-contents)**
### Open/Closed Principle (OCP)
As stated by Bertrand Meyer, "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification." What does that mean though? This principle basically states that you should allow users to add new functionalities without changing existing code.
**Bad:**
```ts
class AjaxAdapter extends Adapter {
constructor() {
super();
}
// ...
}
class NodeAdapter extends Adapter {
constructor() {
super();
}
// ...
}
class HttpRequester {
constructor(private readonly adapter: Adapter) {
}
async fetch<T>(url: string): Promise<T> {
if (this.adapter instanceof AjaxAdapter) {
const response = await makeAjaxCall<T>(url);
// transform response and return
} else if (this.adapter instanceof NodeAdapter) {
const response = await makeHttpCall<T>(url);
// transform response and return
}
}
}
function makeAjaxCall<T>(url: string): Promise<T> {
// request and return promise
}
function makeHttpCall<T>(url: string): Promise<T> {
// request and return promise
}
```
**Good:**
```ts
abstract class Adapter {
abstract async request<T>(url: string): Promise<T>;
// code shared to subclasses ...
}
class AjaxAdapter extends Adapter {
constructor() {
super();
}
async request<T>(url: string): Promise<T>{
// request and return promise
}
// ...
}
class NodeAdapter extends Adapter {
constructor() {
super();
}
async request<T>(url: string): Promise<T>{
// request and return promise
}
// ...
}
class HttpRequester {
constructor(private readonly adapter: Adapter) {
}
async fetch<T>(url: string): Promise<T> {
const response = await this.adapter.request<T>(url);
// transform response and return
}
}
```
**[⬆ back to top](#table-of-contents)**
### Liskov Substitution Principle (LSP)
This is a scary term for a very simple concept. It's formally defined as "If S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e., objects of type S may substitute objects of type T) without altering any of the desirable properties of that program (correctness, task performed, etc.)." That's an even scarier definition.
The best explanation for this is if you have a parent class and a child class, then the parent class and child class can be used interchangeably without getting incorrect results. This might still be confusing, so let's take a look at the classic Square-Rectangle example. Mathematically, a square is a rectangle, but if you model it using the "is-a" relationship via inheritance, you quickly get into trouble.
**Bad:**
```ts
class Rectangle {
constructor(
protected width: number = 0,
protected height: number = 0) {
}
setColor(color: string): this {
// ...
}
render(area: number) {
// ...
}
setWidth(width: number): this {
this.width = width;
return this;
}
setHeight(height: number): this {
this.height = height;
return this;
}
getArea(): number {
return this.width * this.height;
}
}
class Square extends Rectangle {
setWidth(width: number): this {
this.width = width;
this.height = width;
return this;
}
setHeight(height: number): this {
this.width = height;
this.height = height;
return this;
}
}
function renderLargeRectangles(rectangles: Rectangle[]) {
rectangles.forEach((rectangle) => {
const area = rectangle
.setWidth(4)
.setHeight(5)
.getArea(); // BAD: Returns 25 for Square. Should be 20.
rectangle.render(area);
});
}
const rectangles = [new Rectangle(), new Rectangle(), new Square()];
renderLargeRectangles(rectangles);
```
**Good:**
```ts
abstract class Shape {
setColor(color: string): this {
// ...
}
render(area: number) {
// ...
}
abstract getArea(): number;
}
class Rectangle extends Shape {
constructor(
private readonly width = 0,
private readonly height = 0) {
super();
}
getArea(): number {
return this.width * this.height;
}
}
class Square extends Shape {
constructor(private readonly length: number) {
super();
}
getArea(): number {
return this.length * this.length;
}
}
function renderLargeShapes(shapes: Shape[]) {
shapes.forEach((shape) => {
const area = shape.getArea();
shape.render(area);
});
}
const shapes = [new Rectangle(4, 5), new Rectangle(4, 5), new Square(5)];
renderLargeShapes(shapes);
```
**[⬆ back to top](#table-of-contents)**
### Interface Segregation Principle (ISP)
ISP states that "Clients should not be forced to depend upon interfaces that they do not use.". This principle is very much related to the Single Responsibility Principle.
What it really means is that you should always design your abstractions in a way that the clients that are using the exposed methods do not get the whole pie instead. That also include imposing the clients with the burden of implementing methods that they don’t actually need.
**Bad:**
```ts
interface SmartPrinter {
print();
fax();
scan();
}
class AllInOnePrinter implements SmartPrinter {
print() {
// ...
}
fax() {
// ...
}
scan() {
// ...
}
}
class EconomicPrinter implements SmartPrinter {
print() {
// ...
}
fax() {
throw new Error('Fax not supported.');
}
scan() {
throw new Error('Scan not supported.');
}
}
```
**Good:**
```ts
interface Printer {
print();
}
interface Fax {
fax();
}
interface Scanner {
scan();
}
class AllInOnePrinter implements Printer, Fax, Scanner {
print() {
// ...
}
fax() {
// ...
}
scan() {
// ...
}
}
class EconomicPrinter implements Printer {
print() {
// ...
}
}
```
**[⬆ back to top](#table-of-contents)**
### Dependency Inversion Principle (DIP)
This principle states two essential things:
1. High-level modules should not depend on low-level modules. Both should depend on abstractions.
2. Abstractions should not depend upon details. Details should depend on abstractions.
This can be hard to understand at first, but if you've worked with Angular, you've seen an implementation of this principle in the form of Dependency Injection (DI). While they are not identical concepts, DIP keeps high-level modules from knowing the details of its low-level modules and setting them up. It can accomplish this through DI. A huge benefit of this is that it reduces the coupling between modules. Coupling is a very bad development pattern because it makes your code hard to refactor.
DIP is usually achieved by a using an inversion of control (IoC) container. An example of a powerful IoC container for TypeScript is [InversifyJs](https://www.npmjs.com/package/inversify)
**Bad:**
```ts
import { readFile as readFileCb } from 'fs';
import { promisify } from 'util';
const readFile = promisify(readFileCb);
type ReportData = {
// ..
}
class XmlFormatter {
parse<T>(content: string): T {
// Converts an XML string to an object T
}
}
class ReportReader {
// BAD: We have created a dependency on a specific request implementation.
// We should just have ReportReader depend on a parse method: `parse`
private readonly formatter = new XmlFormatter();
async read(path: string): Promise<ReportData> {
const text = await readFile(path, 'UTF8');
return this.formatter.parse<ReportData>(text);
}
}
// ...
const reader = new ReportReader();
const report = await reader.read('report.xml');
```
**Good:**
```ts
import { readFile as readFileCb } from 'fs';
import { promisify } from 'util';
const readFile = promisify(readFileCb);
type ReportData = {
// ..
}
interface Formatter {
parse<T>(content: string): T;
}
class XmlFormatter implements Formatter {
parse<T>(content: string): T {
// Converts an XML string to an object T
}
}
class JsonFormatter implements Formatter {
parse<T>(content: string): T {
// Converts a JSON string to an object T
}
}
class ReportReader {
constructor(private readonly formatter: Formatter) {
}
async read(path: string): Promise<ReportData> {
const text = await readFile(path, 'UTF8');
return this.formatter.parse<ReportData>(text);
}
}
// ...
const reader = new ReportReader(new XmlFormatter());
const report = await reader.read('report.xml');
// or if we had to read a json report
const reader = new ReportReader(new JsonFormatter());
const report = await reader.read('report.json');
```
**[⬆ back to top](#table-of-contents)**
## Testing
Testing is more important than shipping. If you have no tests or an inadequate amount, then every time you ship code you won't be sure that you didn't break anything.
Deciding on what constitutes an adequate amount is up to your team, but having 100% coverage (all statements and branches)
is how you achieve very high confidence and developer peace of mind. This means that in addition to having a great testing framework, you also need to use a good [coverage tool](https://github.com/gotwarlost/istanbul).
There's no excuse to not write tests. There are [plenty of good JS test frameworks](http://jstherightway.org/#testing-tools) with typings support for TypeScript, so find one that your team prefers. When you find one that works for your team, then aim to always write tests for every new feature/module you introduce. If your preferred method is Test Driven Development (TDD), that is great, but the main point is to just make sure you are reaching your coverage goals before launching any feature, or refactoring an existing one.
### The three laws of TDD
1. You are not allowed to write any production code unless it is to make a failing unit test pass.
2. You are not allowed to write any more of a unit test than is sufficient to fail, and; compilation failures are failures.
3. You are not allowed to write any more production code than is sufficient to pass the one failing unit test.
**[⬆ back to top](#table-of-contents)**
### F.I.R.S.T. rules
Clean tests should follow the rules:
- **Fast** tests should be fast because we want to run them frequently.
- **Independent** tests should not depend on each other. They should provide same output whether run independently or all together in any order.
- **Repeatable** tests should be repeatable in any environment and there should be no excuse for why they fail.
- **Self-Validating** a test should answer with either *Passed* or *Failed*. You don't need to compare log files to answer if a test passed.
- **Timely** unit tests should be written before the production code. If you write tests after the production code, you might find writing tests too hard.
**[⬆ back to top](#table-of-contents)**
### Single concept per test
Tests should also follow the *Single Responsibility Principle*. Make only one assert per unit test.
**Bad:**
```ts
import { assert } from 'chai';
describe('AwesomeDate', () => {
it('handles date boundaries', () => {
let date: AwesomeDate;
date = new AwesomeDate('1/1/2015');
assert.equal('1/31/2015', date.addDays(30));
date = new AwesomeDate('2/1/2016');
assert.equal('2/29/2016', date.addDays(28));
date = new AwesomeDate('2/1/2015');
assert.equal('3/1/2015', date.addDays(28));
});
});
```
**Good:**
```ts
import { assert } from 'chai';
describe('AwesomeDate', () => {
it('handles 30-day months', () => {
const date = new AwesomeDate('1/1/2015');
assert.equal('1/31/2015', date.addDays(30));
});
it('handles leap year', () => {
const date = new AwesomeDate('2/1/2016');
assert.equal('2/29/2016', date.addDays(28));
});
it('handles non-leap year', () => {
const date = new AwesomeDate('2/1/2015');
assert.equal('3/1/2015', date.addDays(28));
});
});
```
**[⬆ back to top](#table-of-contents)**
### The name of the test should reveal its intention
When a test fails, its name is the first indication of what may have gone wrong.
**Bad:**
```ts
describe('Calendar', () => {
it('2/29/2020', () => {
// ...
});
it('throws', () => {
// ...
});
});
```
**Good:**
```ts
describe('Calendar', () => {
it('should handle leap year', () => {
// ...
});
it('should throw when format is invalid', () => {
// ...
});
});
```
**[⬆ back to top](#table-of-contents)**
## Concurrency
### Prefer promises vs callbacks
Callbacks aren't clean, and they cause excessive amounts of nesting *(the callback hell)*.
There are utilities that transform existing functions using the callback style to a version that returns promises
(for Node.js see [`util.promisify`](https://nodejs.org/dist/latest-v8.x/docs/api/util.html#util_util_promisify_original), for general purpose see [pify](https://www.npmjs.com/package/pify), [es6-promisify](https://www.npmjs.com/package/es6-promisify))
**Bad:**
```ts
import { get } from 'request';
import { writeFile } from 'fs';
function downloadPage(url: string, saveTo: string, callback: (error: Error, content?: string) => void) {
get(url, (error, response) => {
if (error) {
callback(error);
} else {
writeFile(saveTo, response.body, (error) => {
if (error) {
callback(error);
} else {
callback(null, response.body);
}
});
}
});
}
downloadPage('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', 'article.html', (error, content) => {
if (error) {
console.error(error);
} else {
console.log(content);
}
});
```
**Good:**
```ts
import { get } from 'request';
import { writeFile } from 'fs';
import { promisify } from 'util';
const write = promisify(writeFile);
function downloadPage(url: string, saveTo: string): Promise<string> {
return get(url)
.then(response => write(saveTo, response));
}
downloadPage('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', 'article.html')
.then(content => console.log(content))
.catch(error => console.error(error));
```
Promises supports a few helper methods that help make code more concise:
| Pattern | Description |
| ------------------------ | ----------------------------------------- |
| `Promise.resolve(value)` | Convert a value into a resolved promise. |
| `Promise.reject(error)` | Convert an error into a rejected promise. |
| `Promise.all(promises)` | Returns a new promise which is fulfilled with an array of fulfillment values for the passed promises or rejects with the reason of the first promise that rejects. |
| `Promise.race(promises)`| Returns a new promise which is fulfilled/rejected with the result/error of the first settled promise from the array of passed promises. |
`Promise.all` is especially useful when there is a need to run tasks in parallel. `Promise.race` makes it easier to implement things like timeouts for promises.
**[⬆ back to top](#table-of-contents)**
### Async/Await are even cleaner than Promises
With `async`/`await` syntax you can write code that is far cleaner and more understandable than chained promises. Within a function prefixed with `async` keyword, you have a way to tell the JavaScript runtime to pause the execution of code on the `await` keyword (when used on a promise).
**Bad:**
```ts
import { get } from 'request';
import { writeFile } from 'fs';
import { promisify } from 'util';
const write = util.promisify(writeFile);
function downloadPage(url: string, saveTo: string): Promise<string> {
return get(url).then(response => write(saveTo, response));
}
downloadPage('https://en.wikipedia.org/wiki/Robert_Cecil_Martin', 'article.html')
.then(content => console.log(content))
.catch(error => console.error(error));
```
**Good:**
```ts
import { get } from 'request';
import { writeFile } from 'fs';
import { promisify } from 'util';
const write = promisify(writeFile);
async function downloadPage(url: string): Promise<string> {
const response = await get(url);
return response;
}
// somewhere in an async function
try {
const content = await downloadPage('https://en.wikipedia.org/wiki/Robert_Cecil_Martin');
await write('article.html', content);
console.log(content);
} catch (error) {
console.error(error);
}
```
**[⬆ back to top](#table-of-contents)**
## Error Handling
Thrown errors are a good thing! They mean the runtime has successfully identified when something in your program has gone wrong and it's letting you know by stopping function
execution on the current stack, killing the process (in Node), and notifying you in the console with a stack trace.
### Always use Error for throwing or rejecting
JavaScript as well as TypeScript allow you to `throw` any object. A Promise can also be rejected with any reason object.
It is advisable to use the `throw` syntax with an `Error` type. This is because your error might be caught in higher level code with a `catch` syntax.
It would be very confusing to catch a string message there and would make
[debugging more painful](https://basarat.gitbook.io/typescript/type-system/exceptions#always-use-error).
For the same reason you should reject promises with `Error` types.
**Bad:**
```ts
function calculateTotal(items: Item[]): number {
throw 'Not implemented.';
}
function get(): Promise<Item[]> {
return Promise.reject('Not implemented.');
}
```
**Good:**
```ts
function calculateTotal(items: Item[]): number {
throw new Error('Not implemented.');
}
function get(): Promise<Item[]> {
return Promise.reject(new Error('Not implemented.'));
}
// or equivalent to:
async function get(): Promise<Item[]> {
throw new Error('Not implemented.');
}
```
The benefit of using `Error` types is that it is supported by the syntax `try/catch/finally` and implicitly all errors have the `stack` property which
is very powerful for debugging.
There are also other alternatives, not to use the `throw` syntax and instead always return custom error objects. TypeScript makes this even easier.
Consider the following example:
```ts
type Result<R> = { isError: false, value: R };
type Failure<E> = { isError: true, error: E };
type Failable<R, E> = Result<R> | Failure<E>;
function calculateTotal(items: Item[]): Failable<number, 'empty'> {
if (items.length === 0) {
return { isError: true, error: 'empty' };
}
// ...
return { isError: false, value: 42 };
}
```
For the detailed explanation of this idea refer to the [original post](https://medium.com/@dhruvrajvanshi/making-exceptions-type-safe-in-typescript-c4d200ee78e9).
**[⬆ back to top](#table-of-contents)**
### Don't ignore caught errors
Doing nothing with a caught error doesn't give you the ability to ever fix or react to said error. Logging the error to the console (`console.log`) isn't much better as often it can get lost in a sea of things printed to the console. If you wrap any bit of code in a `try/catch` it means you think an error may occur there and therefore you should have a plan, or create a code path, for when it occurs.
**Bad:**
```ts
try {
functionThatMightThrow();
} catch (error) {
console.log(error);
}
// or even worse
try {
functionThatMightThrow();
} catch (error) {
// ignore error
}
```
**Good:**
```ts
import { logger } from './logging'
try {
functionThatMightThrow();
} catch (error) {
logger.log(error);
}
```
**[⬆ back to top](#table-of-contents)**
### Don't ignore rejected promises
For the same reason you shouldn't ignore caught errors from `try/catch`.
**Bad:**
```ts
getUser()
.then((user: User) => {
return sendEmail(user.email, 'Welcome!');
})
.catch((error) => {
console.log(error);
});
```
**Good:**
```ts
import { logger } from './logging'
getUser()
.then((user: User) => {
return sendEmail(user.email, 'Welcome!');
})
.catch((error) => {
logger.log(error);
});
// or using the async/await syntax:
try {
const user = await getUser();
await sendEmail(user.email, 'Welcome!');
} catch (error) {
logger.log(error);
}
```
**[⬆ back to top](#table-of-contents)**
## Formatting
Formatting is subjective. Like many rules herein, there is no hard and fast rule that you must follow. The main point is *DO NOT ARGUE* over formatting. There are tons of tools to automate this. Use one! It's a waste of time and money for engineers to argue over formatting. The general rule to follow is *keep consistent formatting rules*.
For TypeScript there is a powerful tool called [ESLint](https://typescript-eslint.io/). It's a static analysis tool that can help you improve dramatically the readability and maintainability of your code. There are ready to use ESLint configurations that you can reference in your projects:
- [ESLint Config Airbnb](https://www.npmjs.com/package/eslint-config-airbnb-typescript) - Airbnb style guide
- [ESLint Base Style Config](https://www.npmjs.com/package/eslint-plugin-base-style-config) - a Set of Essential ESLint rules for JS, TS and React
- [ESLint + Prettier](https://www.npmjs.com/package/eslint-config-prettier) - lint rules for [Prettier](https://github.com/prettier/prettier) code formatter
Refer also to this great [TypeScript StyleGuide and Coding Conventions](https://basarat.gitbook.io/typescript/styleguide) source.
### Migrating from TSLint to ESLint
If you are looking for help in migrating from TSLint to ESLint, you can check out this project: <https://github.com/typescript-eslint/tslint-to-eslint-config>
### Use consistent capitalization
Capitalization tells you a lot about your variables, functions, etc. These rules are subjective, so your team can choose whatever they want. The point is, no matter what you all choose, just *be consistent*.
**Bad:**
```ts
const DAYS_IN_WEEK = 7;
const daysInMonth = 30;
const songs = ['Back In Black', 'Stairway to Heaven', 'Hey Jude'];
const Artists = ['ACDC', 'Led Zeppelin', 'The Beatles'];
function eraseDatabase() {}
function restore_database() {}
type animal = { /* ... */ }
type Container = { /* ... */ }
```
**Good:**
```ts
const DAYS_IN_WEEK = 7;
const DAYS_IN_MONTH = 30;
const SONGS = ['Back In Black', 'Stairway to Heaven', 'Hey Jude'];
const ARTISTS = ['ACDC', 'Led Zeppelin', 'The Beatles'];
const discography = getArtistDiscography('ACDC');
const beatlesSongs = SONGS.filter((song) => isBeatlesSong(song));
function eraseDatabase() {}
function restoreDatabase() {}
type Animal = { /* ... */ }
type Container = { /* ... */ }
```
Prefer using `PascalCase` for class, interface, type and namespace names.
Prefer using `camelCase` for variables, functions and class members.
Prefer using capitalized `SNAKE_CASE` for constants.
**[⬆ back to top](#table-of-contents)**
### Function callers and callees should be close
If a function calls another, keep those functions vertically close in the source file. Ideally, keep the caller right above the callee.
We tend to read code from top-to-bottom, like a newspaper. Because of this, make your code read that way.
**Bad:**
```ts
class PerformanceReview {
constructor(private readonly employee: Employee) {
}
private lookupPeers() {
return db.lookup(this.employee.id, 'peers');
}
private lookupManager() {
return db.lookup(this.employee, 'manager');
}
private getPeerReviews() {
const peers = this.lookupPeers();
// ...
}
review() {
this.getPeerReviews();
this.getManagerReview();
this.getSelfReview();
// ...
}
private getManagerReview() {
const manager = this.lookupManager();
}
private getSelfReview() {
// ...
}
}
const review = new PerformanceReview(employee);
review.review();
```
**Good:**
```ts
class PerformanceReview {
constructor(private readonly employee: Employee) {
}
review() {
this.getPeerReviews();
this.getManagerReview();
this.getSelfReview();
// ...
}
private getPeerReviews() {
const peers = this.lookupPeers();
// ...
}
private lookupPeers() {
return db.lookup(this.employee.id, 'peers');
}
private getManagerReview() {
const manager = this.lookupManager();
}
private lookupManager() {
return db.lookup(this.employee, 'manager');
}
private getSelfReview() {
// ...
}
}
const review = new PerformanceReview(employee);
review.review();
```
**[⬆ back to top](#table-of-contents)**
### Organize imports
With clean and easy to read import statements you can quickly see the dependencies of current code. Make sure you apply following good practices for `import` statements:
- Import statements should be alphabetized and grouped.
- Unused imports should be removed.
- Named imports must be alphabetized (i.e. `import {A, B, C} from 'foo';`)
- Import sources must be alphabetized within groups, i.e.: `import * as foo from 'a'; import * as bar from 'b';`
- Prefer using `import type` instead of `import` when importing only types from a file to avoid dependency cycles, as these imports are erased at runtime
- Groups of imports are delineated by blank lines.
- Groups must respect following order:
- Polyfills (i.e. `import 'reflect-metadata';`)
- Node builtin modules (i.e. `import fs from 'fs';`)
- external modules (i.e. `import { query } from 'itiriri';`)
- internal modules (i.e `import { UserService } from 'src/services/userService';`)
- modules from a parent directory (i.e. `import foo from '../foo'; import qux from '../../foo/qux';`)
- modules from the same or a sibling's directory (i.e. `import bar from './bar'; import baz from './bar/baz';`)
**Bad:**
```ts
import { TypeDefinition } from '../types/typeDefinition';
import { AttributeTypes } from '../model/attribute';
import { Customer, Credentials } from '../model/types';
import { ApiCredentials, Adapters } from './common/api/authorization';
import fs from 'fs';
import { ConfigPlugin } from './plugins/config/configPlugin';
import { BindingScopeEnum, Container } from 'inversify';
import 'reflect-metadata';
```
**Good:**
```ts
import 'reflect-metadata';
import fs from 'fs';
import { BindingScopeEnum, Container } from 'inversify';
import { AttributeTypes } from '../model/attribute';
import { TypeDefinition } from '../types/typeDefinition';
import type { Customer, Credentials } from '../model/types';
import { ApiCredentials, Adapters } from './common/api/authorization';
import { ConfigPlugin } from './plugins/config/configPlugin';
```
**[⬆ back to top](#table-of-contents)**
### Use typescript aliases
Create prettier imports by defining the paths and baseUrl properties in the compilerOptions section in the `tsconfig.json`
This will avoid long relative paths when doing imports.
**Bad:**
```ts
import { UserService } from '../../../services/UserService';
```
**Good:**
```ts
import { UserService } from '@services/UserService';
```
```js
// tsconfig.json
...
"compilerOptions": {
...
"baseUrl": "src",
"paths": {
"@services": ["services/*"]
}
...
}
...
```
**[⬆ back to top](#table-of-contents)**
## Comments
The use of a comments is an indication of failure to express without them. Code should be the only source of truth.
> Don’t comment bad code—rewrite it.
> — *Brian W. Kernighan and P. J. Plaugher*
### Prefer self explanatory code instead of comments
Comments are an apology, not a requirement. Good code *mostly* documents itself.
**Bad:**
```ts
// Check if subscription is active.
if (subscription.endDate > Date.now) { }
```
**Good:**
```ts
const isSubscriptionActive = subscription.endDate > Date.now;
if (isSubscriptionActive) { /* ... */ }
```
**[⬆ back to top](#table-of-contents)**
### Don't leave commented out code in your codebase
Version control exists for a reason. Leave old code in your history.
**Bad:**
```ts
type User = {
name: string;
email: string;
// age: number;
// jobPosition: string;
}
```
**Good:**
```ts
type User = {
name: string;
email: string;
}
```
**[⬆ back to top](#table-of-contents)**
### Don't have journal comments
Remember, use version control! There's no need for dead code, commented code, and especially journal comments. Use `git log` to get history!
**Bad:**
```ts
/**
* 2016-12-20: Removed monads, didn't understand them (RM)
* 2016-10-01: Improved using special monads (JP)
* 2016-02-03: Added type-checking (LI)
* 2015-03-14: Implemented combine (JR)
*/
function combine(a: number, b: number): number {
return a + b;
}
```
**Good:**
```ts
function combine(a: number, b: number): number {
return a + b;
}
```
**[⬆ back to top](#table-of-contents)**
### Avoid positional markers
They usually just add noise. Let the functions and variable names along with the proper indentation and formatting give the visual structure to your code.
Most IDE support code folding feature that allows you to collapse/expand blocks of code (see Visual Studio Code [folding regions](https://code.visualstudio.com/updates/v1_17#_folding-regions)).
**Bad:**
```ts
////////////////////////////////////////////////////////////////////////////////
// Client class
////////////////////////////////////////////////////////////////////////////////
class Client {
id: number;
name: string;
address: Address;
contact: Contact;
////////////////////////////////////////////////////////////////////////////////
// public methods
////////////////////////////////////////////////////////////////////////////////
public describe(): string {
// ...
}
////////////////////////////////////////////////////////////////////////////////
// private methods
////////////////////////////////////////////////////////////////////////////////
private describeAddress(): string {
// ...
}
private describeContact(): string {
// ...
}
};
```
**Good:**
```ts
class Client {
id: number;
name: string;
address: Address;
contact: Contact;
public describe(): string {
// ...
}
private describeAddress(): string {
// ...
}
private describeContact(): string {
// ...
}
};
```
**[⬆ back to top](#table-of-contents)**
### TODO comments
When you find yourself that you need to leave notes in the code for some later improvements,
do that using `// TODO` comments. Most IDE have special support for those kinds of comments so that
you can quickly go over the entire list of todos.
Keep in mind however that a *TODO* comment is not an excuse for bad code.
**Bad:**
```ts
function getActiveSubscriptions(): Promise<Subscription[]> {
// ensure `dueDate` is indexed.
return db.subscriptions.find({ dueDate: { $lte: new Date() } });
}
```
**Good:**
```ts
function getActiveSubscriptions(): Promise<Subscription[]> {
// TODO: ensure `dueDate` is indexed.
return db.subscriptions.find({ dueDate: { $lte: new Date() } });
}
```
**[⬆ back to top](#table-of-contents)**
## Translations
This is also available in other languages:
- ![br](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Brazil.png) **Brazilian Portuguese**: [vitorfreitas/clean-code-typescript](https://github.com/vitorfreitas/clean-code-typescript)
- ![cn](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/China.png) **Chinese**:
- [beginor/clean-code-typescript](https://github.com/beginor/clean-code-typescript)
- [pipiliang/clean-code-typescript](https://github.com/pipiliang/clean-code-typescript)
- ![fr](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/France.png) **French**: [ralflorent/clean-code-typescript](https://github.com/ralflorent/clean-code-typescript)
- ![de](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Germany.png) **German**: [mheob/clean-code-typescript](https://github.com/mheob/clean-code-typescript)
- ![ja](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Japan.png) **Japanese**: [MSakamaki/clean-code-typescript](https://github.com/MSakamaki/clean-code-typescript)
- ![ko](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/South-Korea.png) **Korean**: [738/clean-code-typescript](https://github.com/738/clean-code-typescript)
- ![ru](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Russia.png) **Russian**: [Real001/clean-code-typescript](https://github.com/Real001/clean-code-typescript)
- ![es](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Spain.png) **Spanish**: [3xp1o1t/clean-code-typescript](https://github.com/3xp1o1t/clean-code-typescript)
- ![tr](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Turkey.png) **Turkish**: [ozanhonamlioglu/clean-code-typescript](https://github.com/ozanhonamlioglu/clean-code-typescript)
- ![vi](https://raw.githubusercontent.com/gosquared/flags/master/flags/flags/shiny/24/Vietnam.png) **Vietnamese**: [hoangsetup/clean-code-typescript](https://github.com/hoangsetup/clean-code-typescript)
References will be added once translations are completed.
Check this [discussion](https://github.com/labs42io/clean-code-typescript/issues/15) for more details and progress.
You can make an indispensable contribution to *Clean Code* community by translating this to your language.
**[⬆ back to top](#table-of-contents)**
# react-philosophies
[![Epic React Exercises](https://img.shields.io/badge/Epic%20-React%20Exercises-orange.svg?logo=react&color=0abde3)](https://github.com/mithi/epic-react-exercises) [![buy me coffee](https://img.shields.io/badge/Buy%20me%20-coffee!-orange.svg?logo=buy-me-a-coffee&color=795548)](https://ko-fi.com/minimithi) [![PRs welcome!](https://img.shields.io/badge/%20📝%20Contributions-welcome-orange.svg?style=flat)](https://github.com/mithi/react-philosophies/issues) ![Forever a work in progress!](https://img.shields.io/badge/%20🚧%20Forever%20🚧%20%20-under%20construction-yellow.svg) [![Stand With Ukraine](https://raw.githubusercontent.com/vshymanskyy/StandWithUkraine/main/badges/StandWithUkraine.svg)](https://stand-with-ukraine.pp.ua)
If `react-philosophies` helped you in some way, consider [buying me a few cups of coffee ☕☕☕](https://ko-fi.com/minimithi). This motivates me to create more `React` "stuff"! 🙂
> You have to think about what is the right way, even when you have the right idea of what the building blocks should be, there is huge flexibility in how you decide to put the whole system together. It is a craft... and it has a lot to do with valuing simplicity over complexity. Many people do have a tendency to make things more complicated than they need to be... The more stuff you throw into a system, the more complicated it gets and the more likely it is not going to work properly. - Barbara Liskov
## Translations to other languages
- [Chinese](https://github.com/Leecason/react-philosophies-cn) by [@Leecason](https://github.com/Leecason) [![Commit snapshot](https://img.shields.io/badge/v1.0.1-(d1984fd)-orange.svg?color=purple)](https://github.com/mithi/react-philosophies/blob/v1.0.1/README.md)
- [Korean](https://github.com/lim-jiwoo/react-philosophies-ko) by [@lim-jiwoo](https://github.com/lim-jiwoo) [![Commit snapshot](https://img.shields.io/badge/v1.0.2-(fc49813)-orange.svg?color=purple)](https://github.com/mithi/react-philosophies/blob/v1.0.2/README.md)
## Table of Contents
0. [Introduction](#-0-introduction)
1. [The Bare Minimum](#-1-the-bare-minimum)
2. [Design for happiness](#-2-design-for-happiness)
3. [Performance tips](#-3-performance-tips)
4. [Testing principles](#-4-testing-principles)
5. [Insights shared by others](#-5-insights-shared-by-others)
<details>
<summary>The way this document is organized</summary>
<br/>
It was actually difficult for me to separate my thoughts into the `design`, `performance`, and `testing`. I noticed that lot of designs intended for maintainability also make your application faster and easier to test. Apologies if the discussion appears to be cluttered at times.
<br/>
</details>
<details>
<summary><strong>Thanks for all the PRs 🚜, coffee ☕, recommended readings 📚, and sharing of ideas 💡 (View contributors)</strong></summary>
---
#### ❤️ Comments, suggestions, violent reactions? I'd love to hear them!
If there's something that you think should be part of my reading list or/and if you have great ideas that you think I should include here, don't hesitate to submit a PR or an issue. Particularly, I've included the section [`Insights shared by others`](#-5-insights-shared-by-others), should you wish to add your own ideas 🙂. Broken links, grammar, formatting, and typographical error corrections are also welcome. Any contribution to improve `react-philosophies` whether big or small is always appreciated.
---
#### ❤️ Special thanks to those who took the time contribute!
_Note, the following is not an exhaustive list, if you've contributed to this project and you don't see your name included, feel free to submit an PR that adds your name here. Thanks!_
**💡 Comments and suggestions**
- The [`r/reactjs`](https://www.reddit.com/r/reactjs/comments/pvwb6m/what_i_think_about_when_i_write_code_in_react) community
- [Josh W Comeau](https://www.joshwcomeau.com/)
- [@unpunnyfuns](https://github.com/unpunnyfuns)
- [@cassidoo](https://github.com/cassidoo)
**☕ Coffee!**
- [Myles](https://github.com/myles-banner)
- [Daniel](https://Ko-fi.com/home/coffeeshop?txid=7d71404a-fa0c-419d-9808-46ea520c913f&mode=public&img=ogsomeoneboughtme)
- [Ankit](https://github.com/ankitwww)
**🚜 Pull Requests**
- [@fengzilong](https://github.com/fengzilong)
- [@ankitwww](https://github.com/ankitwww)
- [@dzakki](https://github.com/dzakki)
- [@metonym](https://github.com/metonym)
- [@sapegin](https://github.com/sapegin)
**📚 Suggested Readings**
- [ryanmcdermott/clean-code-javascript](https://github.com/ryanmcdermott/clean-code-javascript), [3rs-of-software-architecture](https://github.com/ryanmcdermott/3rs-of-software-architecture), [ryanmcdermott/code-review-tips](https://github.com/ryanmcdermott/code-review-tips)
- [Ben Moseley and Peter Marks: Out of the Tar Pit (2006)](http://curtclifton.net/papers/MoseleyMarks06a.pdf), recommended by [@icyjoseph](https://github.com/icyjoseph)
- [`goldbergyoni/nodebestpractices`](https://github.com/goldbergyoni/nodebestpractices), recommended by [@rstacruz](https://github.com/rstacruz)
- [Dan Abramov: Writing Resilient Components](https://overreacted.io/writing-resilient-components/), recommended by [Mon Quindoza](https://ph.linkedin.com/in/monquindoza)
- [Matthieu Kern: Stop checking if your component is mounted](https://medium.com/doctolib/react-stop-checking-if-your-component-is-mounted-3bb2568a4934), recommended by [@Pierre-CLIND](https://github.com/Pierre-CLIND)
</details>
## 🧘 0. Introduction
`react-philosophies` is:
- things I think about when I write `React` code.
- at the back of my mind whenever I review someone else's code or my own
- just guidelines and NOT rigid rules
- a living document and will evolve over time as my experience grows
- mostly techniques which are variations of basic [refactoring](https://en.wikipedia.org/wiki/Code_refactoring) methods, [SOLID](https://en.wikipedia.org/wiki/SOLID) principles, and [extreme programming](https://en.wikipedia.org/wiki/Extreme_programming) ideas... just applied to `React` specifically 🙂
`react-philosophies` is inspired by various places I've stumbled upon at different points of my coding journey.
Here are a few of them:
- [Sandi Metz](https://sandimetz.com/)
- [Kent C Dodds](https://kentcdodds.com)
- [Zen of Python (PEP 20)](https://www.python.org/dev/peps/pep-0020/), [Zen of Go](https://dave.cheney.net/2020/02/23/the-zen-of-go)
- [trekhleb/state-of-the-art-shitcode](https://github.com/trekhleb/state-of-the-art-shitcode), [droogans/unmaintainable-code](https://github.com/Droogans/unmaintainable-code), [sapegin/washingcode-book](https://github.com/sapegin/washingcode-book/)
> As a seasoned developer I have certain quirks, opinions, and common patterns that I fall back on. Having to explain to another person why I am approaching a problem in a particular way is really good for helping me break bad habits and challenge my assumptions, or for providing validation for good problem solving skills. - [Coraline Ada Ehmke](https://where.coraline.codes)
## 🧘 1. The Bare Minimum
### 1.1 Recognize when the computer is smarter than you
1. Statically analyze your code with [`ESLint`](https://eslint.org/). Enable the [`rule-of-hooks`](https://www.npmjs.com/package/eslint-plugin-react-hooks) and `exhaustive-deps` rule to catch `React`-specific errors.
2. Enable ["strict"](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode) mode. It's 2022.
3. [Be honest about your dependencies](https://overreacted.io/a-complete-guide-to-useeffect/#two-ways-to-be-honest-about-dependencies). Fix `exhaustive-deps` warnings / errors on your `useMemo`'s, `useCallback`'s and `useEffect`'s. You can try ["The latest ref pattern"](https://epicreact.dev/the-latest-ref-pattern-in-react) to keep your callbacks always up-to-date without unnecessary rerenders.
4. [Always add keys](https://epicreact.dev/why-react-needs-a-key-prop) whenever you use `map` to display components.
5. [Only call hooks at the top level](https://reactjs.org/docs/hooks-rules.html). Don’t call Hooks inside loops, conditions, or nested functions.
6. Understand the warning "Can't perform state update on unmounted component". [facebook/react/pull/22114](https://github.com/facebook/react/pull/22114)
7. Prevent the ["white screen of death"](https://kentcdodds.com/blog/use-react-error-boundary-to-handle-errors-in-react) by adding several [error boundaries](https://reactjs.org/docs/error-boundaries.html) at different levels of your application. You can also use them to send alerts to an error monitoring service such as [Sentry](https://sentry.io) if you want to. ([Accounting for Failures In React](https://brandondail.com/posts/fault-tolerance-react))
8. There is a reason why errors and warnings are displayed in the console.
9. Remember [`tree-shaking`](https://webpack.js.org/guides/tree-shaking/)!
10. [Prettier](https://prettier.io/) (or an alternative) formats your code for you, giving you consistent formatting every time. You no longer need to think about it!
11. [`Typescript`](https://www.typescriptlang.org/) and frameworks such as [`NextJS`](https://nextjs.org/) make your life so much easier.
12. I highly recommend [Code Climate](https://codeclimate.com/quality/) (or similar) for open-source repositories or if you can afford it. I find that automatically-detected code smells truly motivates me to reduce the technical debts of the application I'm working on!
### 1.2 Code is just a necessary evil
> "The best code is no code at all. Every new line of code you willingly bring into the world is code that has to be debugged, code that has to be read and understood, code that has to be supported." - Jeff Atwood
> "One of my most productive days was throwing away 1000 lines of code." - Eric S. Raymond
See also: [Write Less Code - Rich Harris](https://svelte.dev/blog/write-less-code), [Code is evil - Artem Sapegin](https://github.com/sapegin/washingcode-book/blob/master/manuscript/Code_is_evil.md)
#### 1.2.1 Think first before adding another dependency
Needless to say, the more you add dependencies, the more code you ship to the browser. Ask yourself, are you actually using the features which make a particular library great?
<details>
<summary><strong><em>🙈 Do you really need it?</strong> View examples of dependencies / code you might not need</em></summary>
<br/>
1. Do you really need [`Redux`](https://redux.js.org/)? It's possible. But keep in mind that React is already a [state management library](https://kentcdodds.com/blog/application-state-management-with-react).
2. Do you really need [`Apollo client`](https://www.apollographql.com/docs/react/) ? Apollo client has many awesome features, like manual normalization. However, it will significantly increase your bundle size. If your application only makes use of features that are not unique to Apollo client , consider using a smaller library such as [`react-query`](https://react-query.tanstack.com/comparison) or [`SWR`](https://github.com/vercel/swr) (or none at all).
3. [`Axios`](https://github.com/axios/axios)? Axios is a great library with features that are not easily replicable with native `fetch`. But if the only reason for using Axios is that it has a better looking API, then consider just using a wrapper on top of fetch (such as [`redaxios`](https://github.com/developit/redaxios) or your own). Determine whether or not your application is actually using Axios's best features.
4. [`Decimal.js`](https://github.com/MikeMcl/decimal.js/)? Maybe [Big.js](https://github.com/MikeMcl/big.js/) or [other smaller libraries](https://www.npmtrends.com/big.js-vs-bignumber.js-vs-decimal.js-vs-mathjs) are sufficient.
5. [`Lodash`](https://lodash.com/)/[`underscoreJS`](https://underscorejs.org/)? [you-dont-need/You-Dont-Need-Lodash-Underscore](https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore)
6. [`MomentJS`](https://momentjs.com/)? [you-dont-need/You-Dont-Need-Momentjs](https://github.com/you-dont-need/You-Dont-Need-Momentjs)
7. You might not need `Context` for theming (`light`/`dark` mode), consider using [`css variables`](https://epicreact.dev/css-variables) instead.
8. You might not even need `Javascript`. CSS is powerful. [you-dont-need/You-Dont-Need-JavaScript](https://github.com/you-dont-need/You-Dont-Need-JavaScript)
<br/>
</details>
#### 1.2.2 Don't be clever. YAGNI!
> "What could happen with my software in the future? Oh yeah, maybe this and that. Let’s implement all these things since we are working on this part anyway. That way it’s future-proof."
**Y**ou **A**ren't **G**onna **N**eed **I**t! Always implement things when you actually need them, never when you just foresee that you may need them. The less code the better! ([Martin Fowler: YAGNI](https://martinfowler.com/bliki/Yagni.html), [C2 Wiki: You Arent Gonna Need It!](https://wiki.c2.com/?YouArentGonnaNeedIt))
Related section: [2.4 Duplication is far cheaper than the wrong abstraction](#-24-duplication-is-far-cheaper-than-the-wrong-abstraction)
### 1.3 Leave it better than you found it
**1.3.1 Detect code smells and do something about them if you need to**.
If you recognize that something is wrong, fix it right then and there. But if it's not that easy to fix or you don't have time to fix it at that moment, at least add a comment (`FIXME` or `TODO`) with a concise explanation of the identified problem. Make sure everybody knows it is broken. It shows others that you care and that they should also do the same when they encounter those kinds of things.
<details>
<summary><strong>🙈 View examples of easy-to-catch code smells</strong></summary>
<br/>
- ❌ Methods or functions defined with a large number of arguments
- ❌ Boolean logic that may be hard to understand
- ❌ Excessive lines of code within a single file
- ❌ Duplicate code which is syntactically identical (but may be formatted differently)
- ❌ Functions or methods that may be hard to understand
- ❌ Classes / Components defined with a large number of functions or methods
- ❌ Excessive lines of code within a single function or method
- ❌ Functions or methods with a large number of return statements
- ❌ Duplicate code which is not identical but shares the same structure (e.g. variable names may differ)
</details>
Keep in mind that code smells don't necessarily mean that code should be changed. A code smell just informs you that you might be able to think of a better way to implement the same functionality.
**1.3.2 Merciless Refactoring. Simple is better than complex.**
> Is the CL more complex than it should be? Check this at every level of the CL—are individual lines too complex? Are functions too complex? Are classes too complex? “Too complex” usually means “can’t be understood quickly by code readers.” It can also mean “developers are likely to introduce bugs when they try to call or modify this code.”- [Google Engineering Practices: What to look for in a code review](https://google.github.io/eng-practices/review/reviewer/looking-for.html)
**💁‍♀️ TIP: Simplify [complex conditionals](https://github.com/sapegin/washingcode-book/blob/master/manuscript/Avoid_conditions.md) and exit early if you can.**
<details>
<summary>🙈 Example of early returns </summary>
```tsx
# ❌ Not-so-good
if (loading) {
return <LoadingScreen />
} else if (error) {
return <ErrorScreen />
} else if (data) {
return <DataScreen />
} else {
throw new Error('This should be impossible')
}
# ✅ BETTER
if (loading) {
return <LoadingScreen />
}
if (error) {
return <ErrorScreen />
}
if (data) {
return <DataScreen />
}
throw new Error('This should be impossible')
```
</details>
**💁‍♀️ TIP: Prefer chained higher-order functions over loops**
If there is no discernable performance difference and if possible, replace traditional loops with chained higher-order functions (`map`, `filter`, `find`, `findIndex`, `some`, etc) [Stackoverflow: What is the advantage of using a function over loops?](https://stackoverflow.com/questions/34402198/what-is-the-advantage-of-using-a-function-over-loops)
### 1.4 You can do better
**💁‍♀️ TIP: Remember that you may not need to put your `state` as a dependency because you can pass a callback function instead.**
You don't need to put `setState` (from `useState`) and `dispatch` (from `useReducer`) in your dependency array for hooks like `useEffect` and `useCallback`. ESLint will NOT complain because React guarantees their stability.
<details>
<summary>🙈 View example</summary>
```tsx
❌ Not-so-good
const decrement = useCallback(() => setCount(count - 1), [setCount, count])
const decrement = useCallback(() => setCount(count - 1), [count])
✅ BETTER
const decrement = useCallback(() => setCount(count => (count - 1)), [])
```
</details>
**💁‍♀️ TIP: If your `useMemo` or `useCallback` doesn't have a dependency, you might be using it wrong.**
<details>
<summary>🙈 View example</summary>
<br />
```tsx
❌ Not-so-good
const MyComponent = () => {
const functionToCall = useCallback(x: string => `Hello ${x}!`,[])
const iAmAConstant = useMemo(() => { return {x: 5, y: 2} }, [])
/* I will use functionToCall and iAmAConstant */
}
✅ BETTER
const I_AM_A_CONSTANT = { x: 5, y: 2 }
const functionToCall = (x: string) => `Hello ${x}!`
const MyComponent = () => {
/* I will use functionToCall and I_AM_A_CONSTANT */
}
````
</details>
**💁‍♀️ TIP: Wrapping your custom context with a hook creates a better-looking API**
Not only does it look better, but you also only have to import one thing instead of two.
<details>
<summary>🙈 View example</summary>
<br />
❌ Not-so-good
```tsx
// you need to import two things every time
import { useContext } from "react"
import { SomethingContext } from "some-context-package"
function App() {
const something = useContext(SomethingContext) // looks okay, but could look better
// blah
}
```
✅ Better
```tsx
// on one file you declare this hook
function useSomething() {
const context = useContext(SomethingContext)
if (context === undefined) {
throw new Error('useSomething must be used within a SomethingProvider')
}
return context
}
// you only need to import one thing each time
import { useSomething } from "some-context-package"
function App() {
const something = useSomething() // looks better
// blah
}
```
</details>
**💁‍♀️ TIP: Think about how your component will be used before coding it**
[Writing APIs is hard](http://sweng.the-davies.net/Home/rustys-api-design-manifesto). [`README Driven Development`](https://tom.preston-werner.com/2010/08/23/readme-driven-development.html) is one useful technique to help design better APIs. I'm not saying we should [RDD](https://rathes.me/blog/en/readme-driven-development/) religiously, just saying that the idea behind it is great. I find that when I first write the API (how the component will be used) before implementing it, this usually creates a better designed component than when I don't.
## 🧘 2. Design for happiness
> "Any fool can write code that a computer can understand. Good programmers write code that humans can understand." - Martin Fowler
> "The ratio of time spent reading versus writing is well over 10 to 1. We are constantly reading old code as part of the effort to write new code. So if you want to go fast, if you want to get done quickly, if you want your code to be easy to write, make it easy to read." ― Robert C. Martin [(Not saying I agree with his political views)](https://www.getrevue.co/profile/tech-bullshit/issues/tech-bullshit-explained-uncle-bob-830918)
**TL;DR**
1. 💖 Avoid state management complexity by removing redundant states
2. 💖 Pass the banana, not the gorilla holding the banana and the entire jungle (prefer passing primitives as props)
3. 💖 Keep your components small and simple - the single responsibility principle!
4. 💖 Duplication is far cheaper than the wrong abstraction (avoid premature / inappropriate generalization)
5. Avoid prop drilling by using composition ([Michael Jackson](https://www.youtube.com/watch?v=3XaXKiXtNjw)). `Context` is not the solution for every state sharing problem
6. Split giant `useEffect`s to smaller independent ones ([KCD: Myths about useEffect](https://epicreact.dev/myths-about-useeffect))
7. Extract logic to hooks and helper functions
8. Prefer having mostly primitives as dependencies to `useCallback`, `useMemo`, and `useEffect`
9. Do not put too many dependencies in `useCallback`, `useMemo`, and `useEffect`
10. For simplicity, instead of having many `useState`s, consider using `useReducer` if some values of your state rely on other values of your state and previous state
11. `Context` does not have to be global to your whole app. Put `Context` as low as possible in your component tree. Do this the same way you put variables, comments, states (and code in general) as close as possible to where they're relevant / being used.
### 💖 2.1 Avoid state management complexity by removing redundant states
When you have redundant states, some states may fall out of sync; you may forget to update them given a complex sequence of interactions.
Aside from avoiding synchronization bugs, you'd notice that it's also easier to reason about and require less code.
See also: [KCD: Don't Sync State. Derive It!](https://kentcdodds.com/blog/dont-sync-state-derive-it), [Tic-Tac-Toe](https://epic-react-exercises.vercel.app/react/hooks/1)
##### 🙈 Example 1
<details>
<summary><strong> 📝🖊️ View the business requirement / problem statement </strong></summary>
---
You are tasked to display properties of a right triangle
- the lengths of each of the three sides
- the perimeter
- the area
The triangle is an object with two numbers `{a: number, b: number}` that should be fetched from an API.
The two numbers represent the two shorter sides of a right triangle.
---
</details>
<details>
<summary> ❌ View not-so-good Solution </summary>
```tsx
const TriangleInfo = () => {
const [triangleInfo, setTriangleInfo] = useState<{a: number, b: number} | null>(null)
const [hypotenuse, setHypotenuse] = useState<number | null>(null)
const [perimeter, setPerimeter] = useState<number | null>(null)
const [areas, setArea] = useState<number | null>(null)
useEffect(() => {
fetchTriangle().then(t => setTriangleInfo(t))
}, [])
useEffect(() => {
if(!triangleInfo) {
return
}
const { a, b } = triangleInfo
const h = computeHypotenuse(a, b)
setHypotenuse(h)
const newArea = computeArea(a, b)
setArea(newArea)
const p = computePerimeter(a, b, h)
setPerimeter(p)
}, [triangleInfo])
if (!triangleInfo) {
return null
}
/*** show info here ****/
}
````
</details>
<details>
<summary> ✅ View "better" solution </summary>
```tsx
const TriangleInfo = () => {
const [triangleInfo, setTriangleInfo] = useState<{
a: number;
b: number;
} | null>(null)
useEffect(() => {
fetchTriangle().then((r) => setTriangleInfo(r))
}, []);
if (!triangleInfo) {
return
}
const { a, b } = triangeInfo
const area = computeArea(a, b)
const hypotenuse = computeHypotenuse(a, b)
const perimeter = computePerimeter(a, b, hypotenuse)
/*** show info here ****/
};
```
</details>
##### 🙈 Example 2
<details>
<summary><strong> 📝🖊️ View the business requirement / problem statement </strong></summary>
---
Suppose you are assigned to design a component which:
1. Fetches a list of unique points from an API
2. Includes a button to either sort by `x` or `y` (ascending order)
3. Includes a button to change the `maxDistance` (increase by `10` each time, initial value should be `100`)
4. Only displays the points that are NOT farther than the current `maxDistance` from the origin `(0, 0)`
5. Assume that the list only has 100 items (meaning you don't need to worry about optimization). If you're working with really large numbers of items, you can memoize some computations with `useMemo`.
---
</details>
<details>
<summary> ❌ View a not-so-good Solution </summary>
```tsx
type SortBy = 'x' | 'y'
const toggle = (current: SortBy): SortBy => current === 'x' ? : 'y' : 'x'
const Points = () => {
const [points, setPoints] = useState<{x: number, y: number}[]>([])
const [filteredPoints, setFilteredPoints] = useState<{x: number, y: number}[]>([])
const [sortedPoints, setSortedPoints] = useState<{x: number, y: number}[]>([])
const [maxDistance, setMaxDistance] = useState<number>(100)
const [sortBy, setSortBy] = useState<SortBy>('x')
useEffect(() => {
fetchPoints().then(r => setPoints(r))
}, [])
useEffect(() => {
const sorted = sortPoints(points, sortBy)
setSortedPoints(sorted)
}, [sortBy, points])
useEffect(() => {
const filtered = sortedPoints.filter(p => getDistance(p.x, p.y) < maxDistance)
setFilteredPoints(filtered)
}, [sortedPoints, maxDistance])
const otherSortBy = toggle(sortBy)
const pointToDisplay = filteredPoints.map(
p => <li key={`${p.x}|{p.y}`}>({p.x}, {p.y})</li>
)
return (
<>
<button onClick={() => setSortBy(otherSortBy)}>
Sort by: {otherSortBy}
<button>
<button onClick={() => setMaxDistance(maxDistance + 10)}>
Increase max distance
<button>
Showing only points that are less than {maxDistance} units away from origin (0, 0)
Currently sorted by: '{sortBy}' (ascending)
<ol>{pointToDisplay}</ol>
</>
)
}
````
</details>
<details>
<summary> ✅ View a "better" Solution </summary>
```tsx
// NOTE: You can also use useReducer instead
type SortBy = 'x' | 'y'
const toggle = (current: SortBy): SortBy => current === 'x' ? : 'y' : 'x'
const Points = () => {
const [points, setPoints] = useState<{x: number, y: number}[]>([])
const [maxDistance, setMaxDistance] = useState<number>(100)
const [sortBy, setSortBy] = useState<SortBy>('x')
useEffect(() => {
fetchPoints().then(r => setPoints(r))
}, [])
const otherSortBy = toggle(sortBy)
const filtedPoints = points.filter(p => getDistance(p.x, p.y) < maxDistance)
const pointToDisplay = sortPoints(filteredPoints, sortBy).map(
p => <li key={`${p.x}|{p.y}`}>({p.x}, {p.y})</li>
)
return (
<>
<button onClick={() => setSortBy(otherSortBy)}>
Sort by: {otherSortBy} <button>
<button onClick={() => setMaxDistance(maxDistance + 10)}>
Increase max distance
<button>
Showing only points that are less than {maxDistance} units away from origin (0, 0)
Currently sorted by: '{sortBy}' (ascending)
<ol>{pointToDisplay}</ol>
</>
)
}
````
</details>
### 💖 2.2 Pass the banana, not the gorilla holding the banana and the entire jungle
> You wanted a banana but what you got was a gorilla holding the banana and the entire jungle. - Joe Armstrong
To avoid falling into this trap, it's a good idea to pass mostly primitives (`boolean`, `string`, `number`, etc) types as props. (Passing primitives is also a good idea if you want to use `React.memo` for optimization)
> A component should just know enough to do its job and nothing more. As much as possible, components should be able to collaborate with others without knowing what they are and what they do.
When we do this, the components will be more loosely coupled, the degree of dependency between two components will be lower. Loose coupling makes it easier to change, replace, or remove components without affecting other components. [stackoverflow:2832017](https://stackoverflow.com/questions/2832017/what-is-the-difference-between-loose-coupling-and-tight-coupling-in-the-object-o)
##### 🙈 Example
<details>
<summary><strong> 📝🖊️ View the business requirement / problem statement </strong></summary>
---
Create a `MemberCard` component that displays two components: `Summary` and `SeeMore`.
The `MemberCard` takes in the prop `id` . The `MemberCard` consumes the hook `useMember` which takes in an `id` and returns the corresponding `Member` information.
```ts
type Member = {
id: string
firstName: string
lastName: string
title: string
imgUrl: string
webUrl: string
age: number
bio: string
/****** 100 more fields ******/
}
```
The `SeeMore` component should display the `age` and `bio` of the `member`.
Include a button to toggle between showing and hiding the `age` and `bio` of the `member`.
The `Summary` component displays the picture of the `member`.
It also displays his `title`, `firstName` and `lastName` (e.g. `Mr. Vincenzo Cassano`).
Clicking the `member`'s name should take you to the `member`'s personal site.
The `Summary` component may also have other functionalities.
(Example, whenever this component is clicked...
the font, size of the image, and background color are randomly changed...
for brevity let's call this "the random styling feature")
---
</details>
<details>
<summary>❌ View a not-so-good solution</summary>
```tsx
const Summary = ({ member } : { member: Member }) => {
/*** include "the random styling feature" ***/
return (
<>
<img src={member.imgUrl} />
<a href={member.webUrl} >
{member.title}. {member.firstName} {member.lastName}
</a>
</>
)
}
const SeeMore = ({ member }: { member: Member }) => {
const [seeMore, setSeeMore] = useState<boolean>(false)
return (
<>
<button onClick={() => setSeeMore(!seeMore)}>
See {seeMore ? "less" : "more"}
</button>
{seeMore && <>AGE: {member.age} | BIO: {member.bio}</>}
</>
)
}
const MemberCard = ({ id }: { id: string })) => {
const member = useMember(id)
return <><Summary member={member} /><SeeMore member={member} /></>
}
````
</details>
<details>
<summary>✅ View a "better" solution</summary>
```tsx
const Summary = ({ imgUrl, webUrl, header }: { imgUrl: string, webUrl: string, header: string }) => {
/*** include "the random styling feature" ***/
return (
<>
<img src={imgUrl} />
<a href={webUrl}>{header}</a>
</>
)
}
const SeeMore = ({ componentToShow }: { componentToShow: ReactNode }) => {
const [seeMore, setSeeMore] = useState<boolean>(false)
return (
<>
<button onClick={() => setSeeMore(!seeMore)}>
See {seeMore ? "less" : "more"}
</button>
{seeMore && <>{componentToShow}</>}
</>
)
}
const MemberCard = ({ id }: { id: string }) => {
const { title, firstName, lastName, webUrl, imgUrl, age, bio } = useMember(id)
const header = `${title}. ${firstName} ${lastName}`
return (
<>
<Summary {...{ imgUrl, webUrl, header }} />
<SeeMore componentToShow={<>AGE: {age} | BIO: {bio}</>} />
</>
)
}
````
</details>
Notice that in the `✅ "better" solution"`, `SeeMore` and `Summary` are components that can be used not just by `Member`. It can be used perhaps by other objects such as `CurrentUser`, `Pet`, `Post`... anything that needs those specific functionality.
### 💖 2.3 Keep your components small and simple
**What is the single responsibility principle?**
> A component should have one and **only one** job. It should do the smallest possible useful thing. It only has responsibilities that fulfill its purpose.
A component with various responsibilities is difficult to reuse. If you want to reuse some but not all of its behavior, it's almost always impossible to just get what you need. It is also likely to be entangled with other code. Components that do one thing which isolate that thing from the rest of your application allows change without consequence and reuse without duplication.
**How to know if your component has a single responsibility?**
> Try to describe that component in one sentence. If it is only responsible for one thing then it should be simple to describe. If it uses the word ‘and’ or ‘or’ then it is likely that your component fails this test.
Inspect the component's state, the props and hooks it consumes, as well as variables and methods declared inside the component (there shouldn't be too many). Ask yourself: Do these things actually work together to fulfill the component's purpose? If some of them don't, consider moving those somewhere else or breaking down your big component into smaller ones.
(The paragraphs above are based on my 2015 article: [Three things I learned from Sandi Metz’s book as a non-Ruby programmer](https://medium.com/@mithi/review-sandi-metz-s-poodr-ch-1-4-wip-d4daac417665))
##### 🙈 Example
<details>
<summary><strong> 📝🖊️ View the business requirement / problem statement </strong></summary>
---
The requirement is to display special kinds of buttons you can click to shop for items of a specific category. For example, the user can select bags, chairs, and food.
- Each button opens a modal you can use to select and "save" items
- If the user has "saved" selected items in a specific category, that category said to be "booked"
- If it is booked, the button will have a checkmark
- You should be able to edit your booking (add or delete items) even if that category is booked
- If the user is hovering the button it should also display `WavingHand` component
- A button should also be disabled when no items for that specific category is available
- When a user hovers a disabled button, a tooltip should show "Not Available"
- If the category has no items available, the button's background should be `grey`
- If the category is booked, the button's background should be `green`
- If the category has available items and is not booked, the button's background should be `red`
- For each category, its corresponding button has a unique label and icon
---
</details>
<details>
<summary>❌ View a not-so-good solution</summary>
```tsx
type ShopCategoryTileProps = {
isBooked: boolean
icon: ReactNode
label: string
componentInsideModal?: ReactNode
items?: {name: string, quantity: number}[]
}
const ShopCategoryTile = ({
icon,
label,
items
componentInsideModal,
}: ShopCategoryTileProps ) => {
const [openDialog, setOpenDialog] = useState(false)
const [hover, setHover] = useState(false)
const disabled = !items || items.length === 0
return (
<>
<Tooltip title="Not Available" show={disabled}>
<StyledButton
className={disabled ? "grey" : isBooked ? "green" : "red" }
disabled={disabled}
onClick={() => disabled ? null : setOpenDialog(true) }
onMouseEnter={() => disabled ? null : setHover(true)}
onMouseLeave={() => disabled ? null : setHover(false)}
>
{icon}
<StyledLabel>{label}<StyledLabel/>
{!disabled && isBooked && <FaCheckCircle/>}
{!disabled && hover && <WavingHand />}
</StyledButton>
</Tooltip>
{componentInsideModal &&
<Dialog open={openDialog} onClose={() => setOpenDialog(false)}>
{componentInsideModal}
</Dialog>
}
</>
)
}
```
</details>
<details>
<summary>✅ View a "better" solution</summary>
```tsx
// split into two smaller components!
const DisabledShopCategoryTile = ({ icon, label }: { icon: ReactNode, label: string }) => {
return (
<Tooltip title="Not available">
<StyledButton disabled={true} className="grey">
{icon}
<StyledLabel>{label}<StyledLabel/>
</Button>
</Tooltip>
)
}
type ShopCategoryTileProps = {
icon: ReactNode
label: string
isBooked: boolean
componentInsideModal: ReactNode
}
const ShopCategoryTile = ({
icon,
label,
isBooked,
componentInsideModal,
}: ShopCategoryTileProps ) => {
const [openDialog, setOpenDialog] = useState(false)
const [hover, setHover] = useState(false)
return (
<>
<StyledButton
disabled={false}
className={isBooked ? "green" : "red"}
onClick={() => setOpenDialog(true) }
onMouseEnter={() => setHover(true)}
onMouseLeave={() => setHover(false)}
>
{icon}
<StyledLabel>{label}<StyledLabel/>
{isBooked && <FaCheckCircle/>}
{hover && <WavingHand />}
</StyledButton>
{openDialog &&
<Dialog onClose={() => setOpenDialog(false)}>
{componentInsideModal}
</Dialog>
}}
</>
)
}
```
</details>
Note: The example above is a simplified version of a component that I've actually seen in production
<details>
<summary>❌ View a not-so-good solution</summary>
```tsx
const ShopCategoryTile = ({
item,
offers,
}: {
item: ItemMap;
offers?: Offer;
}) => {
const dispatch = useDispatch();
const location = useLocation();
const history = useHistory();
const { items } = useContext(OrderingFormContext)
const [openDialog, setOpenDialog] = useState(false)
const [hover, setHover] = useState(false)
const isBooked =
!item.disabled && !!items?.some((a: Item) => a.itemGroup === item.group)
const isDisabled = item.disabled || !offers
const RenderComponent = item.component
useEffect(() => {
if (openDialog && !location.pathname.includes("item")) {
setOpenDialog(false)
}
}, [location.pathname]);
const handleClose = useCallback(() => {
setOpenDialog(false)
history.goBack()
}, [])
return (
<GridStyled
xs={6}
sm={3}
md={2}
item
booked={isBooked}
disabled={isDisabled}
>
<Tooltip
title="Not available"
placement="top"
disableFocusListener={!isDisabled}
disableHoverListener={!isDisabled}
disableTouchListener={!isDisabled}
>
<PaperStyled
disabled={isDisabled}
elevation={isDisabled ? 0 : hover ? 6 : 2}
>
<Wrapper
onClick={() => {
if (isDisabled) {
return;
}
dispatch(push(ORDER__PATH));
setOpenDialog(true);
}}
disabled={isDisabled}
onMouseEnter={() => !isDisabled && setHover(true)}
onMouseLeave={() => !isDisabled && setHover(false)}
>
{item.icon}
<Typography variant="button">{item.label}</Typography>
<CheckIconWrapper>
{isBooked && <FaCheckCircle size="26" />}
</CheckIconWrapper>
</Wrapper>
</PaperStyled>
</Tooltip>
<Dialog fullScreen open={openDialog} onClose={handleClose}>
{RenderComponent && (
<RenderComponent item={item} offer={offers} onClose={handleClose} />
)}
</Dialog>
</GridStyled>
)
}
```
</details>
### 💖 2.4 Duplication is far cheaper than the wrong abstraction
Avoid premature / inappropriate generalization. If your implementation for a simple feature requires a huge overhead, consider other options.
I highly recommend reading [Sandi Metz: The Wrong Abstraction](https://sandimetz.com/blog/2016/1/20/the-wrong-abstraction).
> A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system. Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates might need to be solved in the future. The future problem should be solved once it arrives and you can see its actual shape and requirements in the physical universe. - [Google Engineering Practices: What to look for in a code review](https://google.github.io/eng-practices/review/reviewer/looking-for.html)
See also: [KCD: AHA Programming](https://kentcdodds.com/blog/aha-programming), [C2 Wiki: Contrived Interfaces](https://wiki.c2.com/?ContrivedInterfaces)/[The Expensive Setup Smell](https://wiki.c2.com/?ExpensiveSetUpSmell)/[Premature Generalization](https://wiki.c2.com/?PrematureGeneralization)
## 🧘 3. Performance tips
> Premature optimization is the root of all evil - Tony Hoare
> One accurate measurement is worth a thousand expert opinions. - Grace Hopper
**TL;DR**
1. **If you think it’s slow, prove it with a benchmark.** _"In the face of ambiguity, refuse the temptation to guess."_ The profiler of [React Developer Tools](https://chrome.google.com/webstore/detail/react-developer-tools/fmkadmapgofadopljbjfkapdkoienihi) (Chrome extension) is your friend!
2. Use `useMemo` mostly just for expensive calculations
3. If you're going to use `React.memo`, `useMemo`, and `useCallback` for reducing re-renders, they shouldn't have many dependencies and the dependencies should be mostly primitive types.
4. Make sure your `React.memo`, `useCallback` or `useMemo` is doing what you think it's doing (is it really preventing rerendering? Can you demonstrate empirically that using them on your case has significant performance gains? [Memoization can sometimes make your app worse](https://kentcdodds.com/blog/usememo-and-usecallback), so keep an eye on that!)
5. Stop punching yourself every time you blink ([fix slow renders before fixing rerenders](https://kentcdodds.com/blog/fix-the-slow-render-before-you-fix-the-re-render))
6. Putting your state as close as possible to where it's being used will not only make your code so much easier to read but it would also make your app faster (state colocation)
7. `Context` should be logically separated, do not add too many values in one context provider. If any of the values of your context changes, all components consuming that context also rerenders even if those components don't use the specific value that was actually changed.
8. You can optimize `context` by separating the `state` and the `dispatch` function
9. Know the terms [`lazy loading`](https://nextjs.org/docs/advanced-features/dynamic-import) and [`bundle/code splitting`](https://reactjs.org/docs/code-splitting.html)
10. Window large lists (with [`tannerlinsley/react-virtual`](https://github.com/tannerlinsley/react-virtual) or similar)
11. A smaller bundle size usually also means a faster app. You can visualize the code bundles you've generated with tools such as [`source-map-explorer`](https://create-react-app.dev/docs/analyzing-the-bundle-size/) or [`@next/bundle-analyzer`](https://www.npmjs.com/package/@next/bundle-analyzer) (for NextJS).
12. If you're going to use a package for your forms, I recommend [`react-hook-forms`](https://react-hook-form.com/). I think it is a great balance of good performance and good developer experience.
<details>
<summary><strong>View selected KCD articles about performance</strong></summary>
- [KCD: State Colocation will make your React app faster](https://kentcdodds.com/blog/state-colocation-will-make-your-react-app-faster)
- [KCD: When to `useMemo` and `useCallback`](https://kentcdodds.com/blog/usememo-and-usecallback)
- [KCD: Fix the slow render before you fix the re-render](https://kentcdodds.com/blog/fix-the-slow-render-before-you-fix-the-re-render)
- [KCD: Profile a React App for Performance](https://kentcdodds.com/blog/profile-a-react-app-for-performance)
- [KCD: How to optimize your context value](https://kentcdodds.com/blog/how-to-optimize-your-context-value)
- [KCD: How to use React Context effectively](https://kentcdodds.com/blog/how-to-use-react-context-effectively)
- [KCD: One React Mistake that is slowing you down](https://epicreact.dev/one-react-mistake-thats-slowing-you-down)
- [KCD: One simple trick to optimize React re-renders](https://kentcdodds.com/blog/optimize-react-re-renders)
</details>
## 🧘 4. Testing principles
> Write tests. Not too many. Mostly integration. - Guillermo Rauch
**TL;DR**
1. Your tests should always resemble the way your software is used
2. Make sure that you're not testing implementation details - things which users do not use, see, or even know about
3. If your tests don't make you confident that you didn't break anything, then they didn't do their (one and only) job
5. You'll know you implemented correct tests when you rarely have to change tests when you refactor code given the same user behavior
5. For the front-end, you don't need 100% code coverage, about 70% is probably good enough. Tests should make you more productive not slow you down. Maintaining tests can slow you down. You get diminishing returns on adding more tests after a certain point
6. I like using [Jest](https://jestjs.io/), [React testing library](https://testing-library.com/docs/react-testing-library/intro/), [Cypress](https://www.cypress.io/), and [Mock service worker](https://github.com/mswjs/msw)
<details>
<summary><strong>View selected KCD articles about testing</strong></summary>
- [KCD: Testing Implementation Details](https://kentcdodds.com/blog/testing-implementation-details)
- [KCD: Stop mocking fetch](https://kentcdodds.com/blog/stop-mocking-fetch)
- [KCD: Common mistakes with React Testing Library](https://kentcdodds.com/blog/common-mistakes-with-react-testing-library)
- [KCD: Making your UI tests resilient to change](https://kentcdodds.com/blog/making-your-ui-tests-resilient-to-change)
- [KCD: Write fewer, longer tests](https://kentcdodds.com/blog/write-fewer-longer-tests)
- [KCD: Write tests. Not too many. Mostly integration.](https://kentcdodds.com/blog/write-tests)
- [KCD: How to know what to test](https://kentcdodds.com/blog/how-to-know-what-to-test)
- [KCD: Common Testing Mistakes](https://kentcdodds.com/blog/common-testing-mistakes)
- [KCD: Why you've been bad about testing](https://kentcdodds.com/blog/why-youve-been-bad-about-testing)
- [KCD: Demystifying Testing](https://kentcdodds.com/blog/demystifying-testing)
- [KCD: UI Testing Myths](https://kentcdodds.com/blog/ui-testing-myths)
- [KCD: Effective Snapshot Testing](https://kentcdodds.com/blog/effective-snapshot-testing)
</details>
## 🧘 5. Insights shared by others
If you'd like to share some of the things you think about when you write React code that I didn't touch upon, you can submit a PR and add them to this section.
Thanks for taking the time to share your ideas!
### [Josh W Comeau](https://www.joshwcomeau.com/)
> A similar principle I feel much more strongly about is not to pass _setters_ that have too much power. For example, I might pass an `updateEmail` function instead of a `setUser` function. I don't want random components to be able to _change_ things they don't have any business changing.
> Not only should a component have a single responsibility, it should also have a clear spot on the spectrum of abstraction. On one end, we have generic building blocks like `<Button>`, `<Heading>`, `<Modal>`. On the other end, we have one-off templates like `<Homepage>` and `<ContactForm>`. Every component should have a clear spot on this spectrum. A `<Button>` component shoudn't have a _user_ prop, since users are a higher-abstract concept and `Button` is a low-abstract component.
(Note, the above is taken from his response to my email... I really appreciate that he took the time to share his ideas 🙂)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment