From Chaos to Clarity: The JavaScript Refactoring Expedition Unveiled

When my manager suggested fixing our code, I got excited. But then he said we had to follow 25 rules from the industry. It felt tough, but also like a chance to make our code better.

We started because our code had issues, like repeating stuff or being too hard to understand. These issues, called code smells, make our code messy and can cause problems.

By dealing with these code smells, we’re not just meeting rules — we’re making our code stronger. It’s like fixing a house before it falls apart. So, while it’s a challenge, it’s also a way to make things better for us in the long run.

Before we start, we need some rules:

1. No New Functionality: The primary objective of this refactor is to enhance the existing codebase, not to introduce new features or functionalities.
2. Maintain Test Coverage: All the existing tests that have previously passed should continue to do so after the refactor.
3. Keep the Way Code Is Set Up: It’s important to hold onto how the code is arranged and organized so it stays consistent and easy to manage. But remember, when you’re fixing up code, you might need to divide it into different parts or rename things to keep it looking good and working well.
4. Prefer Breaking Down Changes: It’s preferable to break changes into smaller pieces whenever possible but recognize that it may not always be feasible.
5. Collaboration and Code Review: Encourage collaboration among team members and implement a code review process to maintain code quality and gather input from multiple perspectives.

6. Commit every step: Remember, meaningful commit messages are crucial. They should be descriptive enough to convey the purpose of the change without having to dive deep into the code diff.

How do we start?

I separated this refactoring into 4 sections — this makes sense to handle things in a specific order.

1. Error handling

error handling is an integral part of any software development process, particularly during refactoring. When refactoring code, you’re essentially restructuring it without altering its external behavior.

1. Validate your schema: Instead of integrating validation directly into your code, consider utilizing an external package such as AJV to validate your schema

AJV docs: https://ajv.js.org/

Here’s an example schema:

{
  "type": "object",
  "properties": {
    "name": {
      "type": "string"
    },
    "age": {
      "type": "number"
    },
    "email": {
      "type": "string",
      "format": "email"
    }
  },
  "required": ["name", "age"]
}

Explanation of the schema:

• This schema is for validating an object.
• It requires the object to have a “name” and “age” property.
• The “name” property should be of type string.
• The “age” property should be of type number.
• Optionally, it can have an “email” property which should be a string and should match the format of an email address.

2. Consider Adding Try-Catch: When encountering sections of code lacking a try-catch block, consider adding one. While try-catch blocks are indeed valuable for managing unexpected issues, it’s not necessary to wrap every single code block with one. Overusing try-catch blocks can clutter code. Instead, focus on wrapping try-catch where you anticipate exceptions and have planned to handle them.

3. Async/await: Using async-await simplifies expressing asynchronous flows by making asynchronous code resemble synchronous code, resulting in more concise syntax and enabling easy integration of try-catch for error handling. This approach has largely replaced callbacks and promises due to its efficiency. Incorporating async-await into your codebase is a significant enhancement for code readability, serving as a valuable gift to anyone reading the code.

const mixIngredients = () => {
  return new Promise((resolve) => {
    setTimeout(() => {
      console.log('mix ingredients')
      resolve()
    }, 3000)
  })
}

const bakeCake = async () => {
  return new Promise((resolve) => {
    setTimeout(() => {
      console.log('bake a cake')
      resolve()
    }, 3000)
  })
}

// Do not 👎
const makeCake = () => {
  return mixIngredients()
    .then(bakeCake)
    .catch((error) => logger.error('Failed to make a cake', { errorMessage: error.message }))
}

// Do 👍
const makeCake = async () => {
  try {
    await mixIngredients()
    await bakeCake()
  } catch (error) {
    logger.error('Failed to make a cake', { errorMessage: error.message })
  }
}

2. Clean Code

clean code is the cornerstone of maintainable and efficient software development. When it comes to refactoring, it’s all about improving the structure, readability, and organization of existing code without altering its external behavior

1. Add Entry point: It’s a good idea not to directly take data from a file. Instead, create a starting point that oversees and manages how the functions or data export from one point.

// Do not 👎

// Import function from utils folder
const { someHelper } = require('../../utils/someHelper')

// Do 👍

// Import function from utils folder
const { someHelper } = require('../../utils')

// Create index.js within utils folder
const { someHelper } = require('./someHelper')
module.exports = {
  someHelper,
}

2. Remove Unused or unnecessary segments of code: It’s really important to locate and get rid of any parts of your code that you don’t use or that aren’t necessary. For example, stuff you prepare for combining, making sure your code works for the future, or code that doesn’t match what the product needs.

3. Remove redundant or uninformative log entries: Remove unnecessary or adjust informative log entries to make the logs more informative and useful.

// Do not 👎

const addUser = async (userData) => {
  try {
    await createUserInDB(userData)
  } catch (err) {
    logeer.error('Failed to create user')
  }
}

// Do 👍
const addUser = async (userData) => {
  try {
    await createUserInDB(userData)
  } catch (err) {
    logeer.error('Failed to create user', { errorMessage: err.message })
  }
}

4. Reduce large functions: Dividing complicated logic into smaller, more specific functions makes it more readable. Additionally, consider rewording or rephrasing the functions to enhance clarity and comprehension.

// Do 👍
const authUserWithSSO = async (user) => {
  try {
    const user = this.findUserByEmail(user.email)

    if (!user) {
      // This function will handle all the logic instead of writing it here.
      return this.newUserSSOAuthHandler(user)
    } else {
      // This function will handle all the logic instead of writing it here
      return this.existingUserSSOAuthHandler(user)
    }
  } catch (err) {
    logger.error(`Failed to authenticate user with email ${user.email} using SSO`)
    throw err
  }
}

5. Make sure there is a single ‘Return Point’ in each function: This involves organizing the logic within the function to ensure there’s one exit point, improving code readability, and reducing complexity

// Do not 👎
const existingUserSSOAuthHandler = (user) => {
  if (this.isEmailInBlackList(user.email)) {
    return null
  }

  const shouldHaveAdminRoles = this.isAdminUser(user.email)

  if (shouldHaveAdminRoles) {
    return this.updateUserRolesForAdmin()
  } else {
    return this.updateUserRolesForReadOnly()
  }
}

// Do 👍

const existingUserSSOAuthHandler = (user) => {
  const isEmailInBlacklist = this.isEmailInBlacklist(user.email
  const shouldHaveAdminRoles = this.isAdminUser(user.email)
  const userWithRoles = shouldHaveAdminRoles ? this.updateUserRolesForAdmin(user) : this.updateUserRolesForReadOnly(user)

  return isEmailInBlacklist ? null : userWithRoles
}

6. Meaningful names to variables and functions: When you give your variables names that make sense and explain what they’re for, it helps people understand your code better. Instead of using names that don’t explain much, use names that tell exactly what the variable is used for. This makes it easier for others (and yourself!) to follow along with what the code is doing.

// Do not 👎
const getRolesIds = (validatedRoles) => {
  return validatedRoles.map(role => role.id)
}

// Do 👍
const formatRolesObjectToRolesIDs = (validatedRoles) => {
 return validatedRoles.map(role => role.id)
}

7. Prioritizing the use of const over let : When a variable won’t be reassigned, using const ensures that the variable remains constant throughout the code. This not only adds clarity but also helps prevent accidental reassignments or unintended changes to the variable's value

8. Global constants: Those values that never change throughout your code, are usually named in a specific way to make them stand out. Using ALL_CAPITAL_LETTERS_WITH_UNDERSCORES helps everyone know these are constants, making your code clearer and easier to understand

// Do not - From here we can't understand that admin role ID is global const. 👎
const isHasAdminRole = role.id !=== adminRoleId

// Do -  👍
const isHasAdminRole = role.id !=== ADMIN_ROLE_ID

9. Avoid modifying a reference object: When dealing with collections or complex objects, prefer creating a copy instead of directly modifying the original. This ensures the original remains unchanged.

Note: it’s preferable to clone the object, but be mindful of the size of the object being cloned, as it can lead to excessive memory usage.

// Do not 👎
const processPayment = (paymentData, amount) => {
  paymentData.amount -= amount 
}

const main = () => {
  const payment = { amount: 100 }
  processPayment(payment, 20)
  logger.info(`Remaining amount: ${payment.amount}`)
}

// Do 👍
const processPayment = (paymentData, amount) => {
  return {
    ...paymentData,
    amount: paymentData.amount - amount,
  }
}

const main = () => {
  const payment = { amount: 100 }
  const updatedPayment = processPayment(payment, 20)
  logger.info(`Remaining amount: ${updatedPayment.amount}`)
}

10. Arguments of functions: Minimizing the number of arguments that the function expects is a great practice. It simplifies the function’s interface, making it easier to understand and use

// Do not 👎
const createProduct = (name, price, category, description, manufacturer, weight, inStock) => {
  // Function logic to create a product
}

createProduct('Chair', 49.99, 'Furniture', 'Comfortable chair for home or office', 'XYZ Furniture Co.', '15kg', true)

// Do 👍
const createProduct = ({ name, price, category, description, manufacturer, weight, inStock }) => {
  // Function logic to create a product
}

const productDetails = {
  name: 'Chair',
  price: 49.99,
  category: 'Furniture',
  description: 'Comfortable chair for home or office',
  manufacturer: 'XYZ Furniture Co.',
  weight: '15kg',
  inStock: true,
}

createProduct(productDetails)

11. Consistency in coding conventions: It’s really important to keep things clear and easy to read. Once you pick a style, naming way, or structure, stick with it across all your code.

We have a UserService object:

const UserService = {}

UserService.getUserDataByEmail = function getUserDataByEmail(email) {
  // Logic to get user data by email
}

UserService.getUsersBlackList = function getUsersBlackList() {
  // Logic to get a user's blacklist.
}

We want to add a new function to get a user’s whitelist

const UserService = {}

UserService.getUserDataByEmail = function getUserDataByEmail(email) {
  // Logic to get user data by email
}

UserService.getUsersDeniedList = function getUsersDeniedList() {
  // Logic to get a user's blacklist.
}

// Do not 👎
UserService.findUsersAllowedList = function findUsersAllowedList() {
  // Logic to get a user's whitelist.
}

// Do 👍
UserService.findUsersAllowedList = function findUsersAllowedList() {
  // Logic to get a user's whitelist.
}

12. The Single Responsibility Principle (SRP): Writing code that’s easy to maintain and understand is crucial. Each function should have one specific job or role to keep things clear and manageable.

// Do not 👎
const processDataAndSaveToDatabase = (newEvent) => {
  // Process the data
  const processedData = data.map((item) => {
    return {
      id: item.id,
      name: item.name.toUpperCase(),
      value: item.value * 2,
    }
  })

  // Save to the database
  processedData.forEach((item) => {
    // Database save logic
    // ...
  })
}

const handleNewEvent = (newEvent) => {
  processDataAndSaveToDatabase(newEvent)
}

// Do 👍
const processEvent = (event) => {
  return data.map((item) => {
    return {
      id: item.id,
      name: item.name.toUpperCase(),
      value: item.value * 2,
    }
  })
}

const saveEventToDatabase = (data) => {
  data.forEach((item) => {
    // Database save logic
    // ...
  })
}

const handleNewEvent = (event) => {
  const processedNewEventData = processEvent(event)
  saveEventToDatabase(processedNewEventData)
}

13 Dependency Injection (DI): Dependency Injection (DI) is like organizing your code so that different parts can work independently without being too connected. It helps make the code easier to understand and change because it separates different pieces and lets them get what they need from the outside. This way, it’s easier to test and modify things without causing a mess.

// Do not 👎
class ReportGenerator {
  constructor() {
    this.dataService = new DataService();
  }

  generateReport() {
    const data = this.dataService.fetchData();
    // Generate report logic
  }
}

class DataService {
  fetchData() {
    // Fetch data from an API
  }
}

// Do 👍
class ReportGenerator {
  constructor(dataService) {
    this.dataService = dataService;
  }

  generateReport() {
    const data = this.dataService.fetchData();
    // Generate report logic
  }
}

class DataService {
  fetchData() {
    // Fetch data from an API
  }
}

3. Design Patterns

Design Patterns are a toolkit of solutions to common problems in software design. They provide a structured approach to addressing recurring issues within code. By implementing design patterns, developers can effectively break down complex code into smaller, more manageable components. This not only simplifies maintenance but also enhances understanding. Consequently, utilizing these patterns results in cleaner, more readable, and reliable code for all collaborators involved in the development process.

Problem:

class UserService {
  constructor() {
    this.userGroupService = new UserGroupService() 
    this.userRolesService = new UserRolesService() 
  }

  // Methods using userGroupService and userRolesService
  getUserGroups() {
    return this.userGroupService.getUserGroups()
  }

  getUserRoles() {
    return this.userRolesService.getUserRoles()
  }
}


const userService = new UserService()

// Example usage
const userGroups = userService.getUserGroups()
const userRoles = userService.getUserRoles()
// Use userGroups and userRoles as needed in your router logic

1. Dependency on Concrete Implementations: The UserService create instances of UserGroupService and UserRolesService in its constructor, is not a good idea, creating instances within a constructor can mess up encapsulation by mixing classes too closely, which might expose stuff we want to keep private and make it harder to change things later on.
2. Scalability Issues: As the codebase grows, tight coupling makes it harder to maintain and extend. If new services are introduced or modifications are needed, changes might propagate across multiple classes, leading to maintenance challenges and potential bugs.

How to solve it - Factory Method

The Factory Method pattern is all about abstracting how objects are created. It lets subclasses or specific methods handle the job of creating objects. This way, you can create different objects without tying them too closely to your services.

Advantages of the Factory Method:

• Decoupled Object Creation: Separation of object creation logic from class promotes flexibility and easier maintenance.
• Scalable Extension: Easily extendable to create new objects or subclasses without modifying existing classes, enhancing scalability.
• Centralized Creation Logic: Centralizing object creation logic within factory classes simplifies management and modification.

Organized (Decoupled) Approach using Factory Pattern:

// Implementing Services
class UserService {
  constructor(userGroupService, userRolesService) {
    this.userGroupService = userGroupService
    this.userRolesService = userRolesService
  }

  getUserGroups() {
    return this.userGroupService.getUserGroups()
  }

  getUserRoles() {
    return this.userRolesService.getUserRoles()
  }
}

// Implementing Factory
class ServiceFactory {
  static createUserGroupService() {
    return new UserGroupService()
  }

  static createUserRolesService() {
    return new UserRolesService()
  }

  static createUserService() {
    const userGroupService = ServiceFactory.createUserGroupService()
    const userRolesService = ServiceFactory.createUserRolesService()
    return new UserService(userGroupService, userRolesService)
  }
}

// Usage in router using Factory
const userService = ServiceFactory.createUserService() // Using the factory to create UserService

// Example usage
const userGroups = userService.getUserGroups()
const userRoles = userService.getUserRoles()
// Use userGroups and userRoles as needed in your router logic

You can read more about design patterns: https://refactoring.guru/design-patterns

4. TESTS

When you’re fixing up your code, don’t forget about the tests! Clear tests are just as important as clean code. If you change how things work, you’ve gotta update the tests too. Otherwise, things can get confusing, and the whole development process might not work right. So, make sure those tests are clear and match the changes you’re making to keep everything running smoothly.

1. Sandbox: Instead of using Sinon directly for test stuff, try a sandbox — it’s like a special place just for your tests. You can make, manage, and clean up all those fake things you need for testing. It keeps your tests clean and separate, making them easier to understand and manage

const sinon = require('sinon')
const assert = require('assert')

describe('MyFunction', () => {
  let sandbox

  beforeEach(() => {
    sandbox = sinon.createSandbox()
  })

  afterEach(() => {
    sandbox.restore()
  })

  it('should do something with a stub', () => {
    const stub = sandbox.stub(UserModule, 'getUserAge').returns(42)

    // Test logic using the stub
    const userDetails = UserModule.getUserDetails()
    assert.strictEqual(userDetails.age, 42)
    sinon.assert.calledOnce(stub)
  })

  // Other tests utilizing the sandbox for stubbing, mocking, or spying
})

2. Real-world input: Use real-world input data in tests to simulate practical scenarios. This approach ensures comprehensive coverage, validating the code’s behavior under various conditions and improving reliability by identifying edge cases early in development.

Try using a package like faker. This package provides a lot of functions for creating realistic data (https://github.com/faker-js/faker).

// Do not 👎
const mockUser = { email: 'a@a', phone: '123' }

// Do 👍
const mockUser = { email: 'elad@gmail.com', phone: '+972526745321' }

3. AAA pattern: Organize tests following the AAA pattern Arrange, Act, Assert.

1. Arrange — sets up the test environment
2. Act — performs the action
3. Assert — verifies the expected outcome.

This structured approach enhances test readability and clarity while promoting consistent testing practices

describe('Calculator', () => {
  it('should add two numbers correctly', () => {
    // Arrange
    const num1 = 5
    const num2 = 10

    // Act
    const result = add(num1, num2)

    // Assert
    assert.strictEqual(result, 15)
  })

  // Other tests following the AAA pattern
})

4. Remove redundant or duplicated tests: Fix tests that don’t check anything apart from coverage. Also, combine tests that look at the same things. This makes sure our tests are good but not messy

5. Do not catch errors, expect them: Let errors propagate rather than catch them within the test code. Instead, you can use assertion libraries like Chai to explicitly expect and assert specific errors

const { expect } = require('chai')

describe('SomeFunction', () => {
  it('should throw an error with specific message', () => {
    // Arrange
    const areYouAllowedToDrinkAlcohol = (age) => {
      if (age < 18) {
        // Act - This function should throw an error with a specific message
        throw new Error('Not allowed to drink alcohol')
      }
      return true
    }

    // Assert
    expect(areYouAllowedToDrinkAlcohol(15)).to.throw('Not allowed to drink alcohol')
  })

  // Other tests expecting and asserting specific errors
})

6. Tags: label tests with descriptive tags based on their features like what they do, how important they are, or where they’re used. These tags help sort tests, so you can run just the ones you need, making testing easier to manage.

describe('Authentication Service', function () {
  it('should authenticate the user @functional', function () {
    // Test logic for user authentication
  })

  it('should generate a valid JWT token @environment', function () {
    // Test logic for JWT token generation
  })

  it('should reject invalid credentials @functional @priority', function () {
    // Test logic for rejecting invalid credentials
  })
})

7. When to use “afterAll” and “afterEach”:

Use “afterAll” for cleanup tasks that run once after the entire test suite. It’s useful when tasks don’t affect individual test cases or shared resources between them.

Use “afterEach” to reset states or ensure a clean setup after each test. This keeps each test separate, reducing the chance of issues from shared resources between tests.

8. Use at least 2 ‘describe: Using nested ‘describe’ blocks in Mocha helps make tests clearer and more organized. When you group related test cases under different contexts, like various scenarios or conditions, it’s easier to see what each test suite is about. This structured setup makes the test suite clearer and easier to maintain, letting you navigate and understand the tested features better.

describe('Authentication Service', () => {
  describe('When user credentials are valid', () => {
    it('should authenticate the user successfully', () => {
      // Test logic for successful user authentication
    })

    it('should generate a valid JWT token', () => {
      // Test logic for generating a valid JWT token upon successful authentication
    })
  })

  describe('When user credentials are invalid', () => {
    it('should reject authentication for invalid credentials', () => {
      // Test logic for rejecting authentication for invalid credentials
    })

    it('should not generate a JWT token for invalid credentials', () => {
      // Test logic for not generating a JWT token upon invalid authentication
    })
  })

  // Add more describe blocks for additional scenarios if needed
})

Summary:

In short, refactoring has been enjoyable. The key lesson I’ve learned is to break it into smaller parts. This cuts down on potential mistakes and bugs. When you tackle smaller pieces, each focusing on one thing, you can give it more attention, test it thoroughly, and manage changes better. This step-by-step method boosts efficiency, keeps the code stable, and makes the transition to a better codebase much smoother and safer.

LAST WORDS:

IF YOU CAN DREAM IT YOU CAN DO IT

References:

1. Clean Code: Robert C. Martin … Uncle Bob
2. Yoni Goldberg https://github.com/goldbergyoni/nodebestpractices
3. Yoni Goldberg https://github.com/goldbergyoni/javascript-testing-best-practices