Writing Clean Functions

JavaScript

Writing code isn't the hard part, it's the maintaining and scaling it that keeps programmers awake at night. All programs are made up of series of functions, writing clean code is essential to keeping them maintainable and scalable. In this article, I've listed advice from Robert C. Martin's book Clean Code about writing clean functions.

Function should be small

There is no hard and fast rule the fixed number of lines a function should be. A function should be small in the sense that it should be readable, maintainable and testable. Sometimes that's 3 lines, sometimes 15 lines. ‘Small’ in software engineering often means focused and cohesive rather than just few lines of code.

Functions should do one thing, do it well, and do it only

This simply means don’t put all your logic inside one function. Here is an example where a function does multiple thing-

function processUserData(userData) {
 // Thing 1: Validate data
 if (!userData.name || !userData.email) {
   throw new Error('Missing required fields')
 }

 // Thing 2: Calculate age from birthdate  
 const today = new Date()
 const birthDate = new Date(userData.birthDate)
 const age = today.getFullYear() - birthDate.getFullYear()

 // Thing 3: Format phone number
 const phone = userData.phone.replace(/\D/g, '')
 const formattedPhone = `(${phone.slice(0,3)}) ${phone.slice(3,6)}-${phone.slice(6)}`

 return { ...userData, age, phone: formattedPhone }
}

A much cleaner version broken down to functions where each does only one thing.

function processUserData (userData) {
  validateUserData (userData)
  const age = calculateAge (userData.birthDate)
  const formattedPhone = formatPhoneNumber(userData.phone)
  return { ...userData, age, phone: formattedPhone }
}

function validateUserData(userData) {
 if (!userData.name || !userData.email) {
   throw new Error('Missing required fields')
 }
}
 
function calculateAge(birthDate) {
 const today = new Date()
 const birth = new Date(birthDate)
 return today.getFullYear() - birth.getFullYear()
}
 
function formatPhoneNumber(phone) {
 const digits = phone.replace(/\D/g, '')
 return `${digits.slice(0,3)}) ${digits.slice(3,6)}-${digits.slice(6)}`
}

Use Descriptive Names

Function names should clearly describe what it does.

// Bad - unclear names
function calc(d) {
 return d * 24
}
// Good - descriptive names
function convertDaysToHours(days) {
 return days * 24
}

Minimize Function Arguments

Function shouldn't have more than three arguments.

// Bad - too many arguments
function createUser(firstName, lastName, email, phone, address, city, state, zip, country) {
  // implementation
}

// Good - use object parameter
function createUser(userInfo) {
 const { firstName, lastName, email, phone, address } = userInfo
// implementation
}

Avoid Flag Arguments

Boolean flags indicate the function does more than one thing. If you are using flag to control different logic, you shouldn't be using it. Here is an example of bad usage of flag argument.

function processFile(filename, shouldCompress) {
 if (shouldCompress) {
  // Compress then upload
  const compressed = compress(filename);
  upload(compressed)
 } else {
  // Just validate then archive
  validate(filename)
  archive(filename)
 }
}

A cleaner approach would be-

function compressAndUpload(filename) {
  const compressed = compress(filename)
  upload(compressed)
}

function validateAndArchive(filename) {
  validate(filename)
  archive(filename)
}

Flags are fine if it's just for controlling configuration.

// this is fine, not changing entire logic
function formatText(text, uppercase) {
  const cleaned = text.trim()
  return uppercase ? cleaned.toUpperCase() : cleaned
}

Have No Side Effects

This principle focuses on how function behaves. Function shouldn't secretly change things while doing it's specific job.
In the first function below, it's modifying fullName when it's job was to return only name.

// Bad - modifies the input
 function getFullName(user) {
   user.fullName = `${user.firstName} ${user.lastName}` // Side effect!
   return user.fullName
 }
 <br/>
 // Good - doesn't modify input
 function getFullName(user) {
   return `${user.firstName} ${user.lastName}`
 }

Command Query Separation

Functions should either do something (command) or return something (query), not both.
In the example below, addToCart is both pushing data and also returning length. During usage it becomes confusing whether it's checking length or adding item AND checking

function addToCart(item) {
  cart.push(item)
  return cart.length// Returns info while changing state
}
 
 // Usage is confusing:
 if (addToCart(newItem) > 5) {
  showWarning('Cart is getting full')
}

A better approach would be:

// Command - only changes state
 function addToCart(item) {
   cart.push(item)
 }
 
 // Query - only returns information  
 function getCartSize() {
   return cart.length
 }
 
 addToCart(newItem)
  if (getCartSize() > 5) {
   showWarning('Cart is getting full')
 }

Prefer Exceptions to Error Codes

Use exceptions instead of returning error codes for cleaner error handling. Returning error codes gets mix up with normal logic flow.

// Bad
function divide(a, b) {
 if (b === 0) return -1 // Error code
   return a / b
 }
 
 const result = divide(10, 0)
 if (result === -1) { // have to check for error code
   console.log('Error: division by zero')
 } else {
   console.log('Result:', result)
 }

With exceptions, the normal flow stays clean and error handling is separate.

// Good
function divide(a, b) {
  if (b === 0) throw new Error('Division by zero')
  return a / b
}

try {
  const result = divide(10, 2)
  console.log('Result:', result)
} catch (error) {
  console.log('Error:', error.message)
}

Don't Repeat Yourself (DRY)

Eliminate code duplication.

// Bad
 function taxForEmployee(emp) {
   if (emp.type === 'FULL_TIME') return emp.salary * 0.3
   if (emp.type === 'PART_TIME') return emp.salary * 0.15
 }
 
 // Good
 function taxForEmployee(emp) {
   const rates = { FULL_TIME: 0.3, PART_TIME: 0.15 }
   return emp.salary * rates[emp.type]
 }

Single Level of Abstraction

Don't mix abstraction layers in a function, this means keep abstraction layers in same level. For example, when you order a meal in restaurant, the process goes something like this-
browse menu -> select meal -> eat meal -> make payment. You don't have to think about small details like ingredients, cooking or plating.

// Bad - multiple level of abstraction
function orderMeal() {
  browseMenu() // level 1
  selectMeal() // level 1
  checkIngredients() // level 2
  cookFood() // level 2
  plateFood() // level 2
  eatFood() // level 1
  makePayment() // level 1
}

// Good - keep single level of abstraction
function ordeMeal() {
  browseMenu() // level 1
  selectMeal() // level 1
  eatFood() // level 1
  makePayment() // level 1
}
function selectMeal() {
  checkIngredients() // level 2
  cookFood() // level 2
  plateFood() // level 2
}
function makePayment() {
  calculateBill()      // level 2
  processPayment()     // level 2
  provideBillReceipt() // level 2
}