Clean Code: Mastering Your Coding Skills
Naming, guard clauses, SOLID, DRY, error handling and leaky abstractions, with before-and-after examples
Code smells and anti-patterns
Code smells are structures in the code that indicate a violation of fundamental design principles and hurt design quality. The classic is spaghetti code: unstructured, tangled and hard to maintain.
An anti-pattern is a common response to a recurring problem that turns out to be ineffective and counterproductive.
What are we aiming for instead? The code quality factors: readability, complexity, modularity, reusability, testability, extensibility, reliability, performance, security, scalability, usability, portability, and more.
And there’s one measurement that captures most of them:
The only valid measurement of code quality: WTFs per minute.
Naming is everything
“There are only two hard things in Computer Science: cache invalidation and naming things.” (Phil Karlton)
Use descriptive, meaningful names for variables, functions, classes and modules.
Use meaningful and pronounceable names:
// Bad:
const yyyymmdstr = moment().format("YYYY/MM/DD")
// Good:
const currentDate = moment().format("YYYY/MM/DD")Use the same vocabulary for the same type of thing:
// Bad:
getUserInfo()
getUserData()
getUserRecord()
getUserObj()
// Good:
getUser()Use searchable names. Generic names like events, posts or document are impossible to grep for, and some of them shadow browser globals:
// Bad:
const events = getNearByEvents()
const document = getDocument()
// Good:
const nearbyEvents = getNearByEvents()
const doc = getDoc()Don’t add unneeded context. If the object’s name already says it, don’t repeat it in every field:
// Bad:
const Car = {
carMake: "Honda",
carModel: "Accord",
carColor: "Blue"
}
// Good:
const Car = {
make: "Honda",
model: "Accord",
color: "Blue"
}Function names should say what they do:
// Bad: it's hard to tell from the name what is added
addToDate(date, 1)
// Good:
addMonthToDate(1, date)Small things that add up
Limit function parameters. Pass an object, and get named arguments and defaults for free:
// Bad:
function createMenu(title, body, buttonText, cancellable) { }
createMenu("Foo", "Bar", "Baz", true)
// Good:
function createMenu({ title, body, buttonText, cancellable = false }) { }
createMenu({ title: "Foo", body: "Bar", buttonText: "Baz" })Make large numbers readable with numeric separators:
// Bad:
const largeNumber = 1000000000
// Good:
const largeNumber = 1_000_000_000Avoid export default. Named exports keep the same name everywhere they're imported, which makes code searchable and refactors safe:
// Bad:
export default { query, get, post, put, remove }
// Good:
export const storageService = { query, get, post, put, remove }Use strict comparison. Prefer === over ==, which applies surprising type coercions:
0 == false // true
17 == '17' // true
true == 1 && true == '1' // true
0 === false // false
17 === '17' // false
true === 1 // falseFail fast: guard clauses
Handle the exceptional cases at the start of a function, with early returns. It improves readability and removes nested conditionals:
// Bad:
function addNums(x, y) {
if (typeof x === 'number' && typeof y === 'number') {
return x + y
} else if (typeof x !== 'number') {
throw new Error(`${x} is not a number`)
} else {
throw new Error(`${y} is not a number`)
}
}
// Good:
function addNums(x, y) {
if (typeof x !== 'number') throw new Error(`${x} is not a number`)
if (typeof y !== 'number') throw new Error(`${y} is not a number`)
return x + y
}The result: the happy path runs straight down the function, and every exit is handled up front.
SOLID
Single responsibility (S)
Functions should do one thing:
// Bad:
function emailClients(clients) {
clients.forEach(client => {
const clientRecord = database.lookup(client)
if (clientRecord.isActive()) {
sendEmail(client)
}
})
}
// Good:
function isActiveClient(client) {
const clientRecord = database.lookup(client)
return clientRecord.isActive()
}
function emailActiveClients(clients) {
clients.filter(isActiveClient).forEach(sendEmail)
}Open-closed (O)
Software entities (classes, modules, functions) should be open for extension, but closed for modification. Adding a new payment method should mean adding code, not editing a function that already works:
// Bad: every new method means editing this function
function processPayment(price, method, details) {
if (method === 'card') { /* ... */ }
else if (method === 'paypal') { /* ... */ }
}
// Good: new methods plug in from the outside
function processPayment(price, paymentMethod) {
paymentMethod.pay(price)
}Liskov substitution (L)
An object may be replaced by a sub-object without breaking the program.
Any error handler with the same signature can be swapped in:
function makeRequest(url, errorHandler) {
fetch(url)
.then(response => response.json())
.catch(error => errorHandler(error))
}
const consoleErrorHandler = (error) => console.log(error)
const externalErrorHandler = (error) => sendErrorToExternalService(error)
makeRequest(url, consoleErrorHandler)
makeRequest(url, externalErrorHandler)Interface segregation (I)
No class should be forced to implement methods it won’t use:
// Bad:
class Product {
getDetails() { }
print() { }
}
class PhysicalProduct extends Product {
// print() is inherited but not needed
}
// Good:
class Product {
getDetails() { }
}
class PrintableProduct extends Product {
print() { }
}
class PhysicalProduct extends Product { }Dependency inversion (D)
Depend on abstractions, not on concrete implementations:
// Bad:
class PasswordReminder {
constructor() {
this.dbConnection = new MySqlConnection()
}
}
// Good:
class PasswordReminder {
constructor(connection) {
this.dbConnection = connection
}
}That’s also the idea behind dependency injection: an object receives the objects it depends on instead of creating them. Frameworks do it for you, like Spring in Java:
public class Car {
@Autowired
public Car(Engine engine, Transmission transmission) {
this.engine = engine;
this.transmission = transmission;
}
}and ASP.NET Core:
public IActionResult About([FromServices] IDateTime dateTime)
{
return Content($"Current server time: {dateTime.Now}");
}Design principles
Separation of concerns. Achieved through modularization, encapsulation and layering. MVC is the classic example; Vue 3’s Composition API is a modern one, grouping code by feature instead of by option type.
Get Yaron Biton’s stories in your inbox
Join Medium for free to get updates from this writer.
YAGNI: You Aren’t Gonna Need It. Don’t code things “just in case”:
- Focus on immediate needs.
- Avoid over-engineering.
- Keep the codebase lean and maintainable.
- Iterate and refactor when the need is real.
DRY: Don’t Repeat Yourself. Keep the code base small, reuse working code, and recognize the more general idea behind a function. The nemesis of reuse is tight coupling.
Coupling is the degree to which each module relies on the others. Keep it low, even if it costs a few more lines:
- Avoid globals; use good names and scopes.
- Don’t get intimate with other components: don’t rely on their internal structure or algorithms.
- Don’t expect a certain order of function calls.
- Avoid sending and receiving too many parameters.
- Beware of inconsistent objects (not fully loaded, or lazily initialized).
Error handling
Handle errors gracefully, give meaningful messages that help debugging, and never reveal sensitive information in them.
Don’t swallow caught errors:
// Bad:
try {
functionThatMightThrow()
} catch (error) {
console.log(error)
}
// Good:
try {
functionThatMightThrow()
} catch (error) {
console.error(error)
notifyUserOfError(error)
reportErrorToService(error)
}The same goes for rejected promises: .catch(err => console.log(err)) is not handling.
Handle nulls precisely with the nullish coalescing assignment:
// Verbose:
if (user.name === null || user.name === undefined) {
user.name = 'Anonymous'
}
// Catches too much: replaces '', 0 and false too
user.name = user.name || 'Anonymous'
// Clean and precise:
user.name ??= 'Anonymous'Coming maybe: the proposed safe-assignment operator ?= (its syntax is still under heavy debate) would replace nested try/catch with a tuple:
const [err, data] ?= await fetch('https://example.com/api')
if (err) console.error('Error:', err)Leaky abstractions
A leaky abstraction is a design flaw where an abstraction meant to hide complexity fails to do so, and its users have to understand what’s underneath anyway.
Axios. It wraps the fetch API with useful methods, but it treats non-2xx responses as errors, while fetch doesn't. To use the abstraction correctly, you need to know how it differs from what it wraps. With plain fetch, you check yourself:
fetch('https://jsonplaceholder.typicode.com/invalid-url')
.then(res => {
if (!res.ok) throw new Error('Network response was not ok ' + res.statusText)
return res.json()
})
.then(data => console.log('Fetch data:', data))
.catch(error => console.error('Fetch error:', error))ORMs. In real life you usually end up writing plain old SQL, for example to fix N+1 queries the ORM generated behind your back:
Design systems built by extending a third-party component library leak in the same way: sooner or later, the library’s internals show through.
Tools that keep code clean
Linters enforce coding standards automatically:
They can be strict, and sometimes opinionated:
Automated tests, unit and integration, ensure quality as the code changes, and a coverage report shows how much of the code they exercise.
TDD. Writing tests before the code clarifies requirements and design, and leads to cleaner code with fewer defects:
E2E tests check the whole app the way a user would:
Summary
- Code smells and anti-patterns
- Naming things
- Failing fast and guard clauses
- SOLID
- Dependency injection
- Separation of concerns
- YAGNI
- Reuse (DRY)
- Decoupling
- Error handling
- Leaky abstractions
- Linters
- Tests, coverage and TDD
And remember the only metric that matters: WTFs per minute.
Yaron Biton, MisterBit CTO
