Simplifying a JavaScript Function With 12 Automated Refactorings

Executing many automated refactorings in a row is a powerful way to improve your code quickly. The advantage of this approach over manual refactoring is that it is less likely to introduce bugs and that it can often be faster with the right keyboard shortcuts. However, it is a bit of an art to chain refactorings, as it can involve unintuitive actions to enable further steps.

This blog post shows an example of how to simplify a small JavaScript function in a series of 12 automated refactorings without changing its behavior. I'll be using Visual Studio Code and the P42 JavaScript Assistant refactoring extension.

💡  This blog post uses P42 JavaScript Assistant v1.55

Initially, the function (from this blog post) looks at follows:

const lineChecker = (line, isFirstLine) => {
  let document = ``;

  if (line !== "" && isFirstLine) {
    document += `<h1>${line}</h1>`;
  } else if (line !== "" && !isFirstLine) {
    document += `<p>${line}</p>`;
  } else if (line === "") {
    document += "<br />";
  }

  return document;
};

After refactoring, the function is much shorter and easier to comprehend:

const lineChecker = (line, isFirstLine) => {
  if (line === "") {
    return `<br />`
  }

  return isFirstLine ? `<h1>${line}</h1>` : `<p>${line}</p>`;
};

Here are the steps that I took to refactor the function:

Simplify Control Flow and Remove Variable

The first refactorings eliminate the document variable and simplify the control flow. This change makes it easier to reason about the function because there is less state (i.e., no document variable) and several execution paths return early.

1. Pull out the + from the += assignments into regular string concatenation. This step enables the introduction of early return statements in the next step.
2. Replace re-assigning the document variable with early return statements. This step simplifies the control flow and enables inlining the document variable.
3. Inline the document variable. This step removes an unnecessary variable and enables the removal of the empty string literals in the next step.
4. Remove empty string literals by merging them into the templates.

After applying these steps, the function looks as follows:

const lineChecker = (line, isFirstLine) => {
  if (line !== "" && isFirstLine) {
    return `<h1>${line}</h1>`;
  } else if (line !== "" && !isFirstLine) {
    return `<p>${line}</p>`;
  } else if (line === "") {
    return `<br />`;
  }

  return ``;
};

Simplify Conditions and Remove Code

The next goals are to simplify the conditions in the if statements and to remove dead or unnecessary code. This change further reduces the complexity of the function and makes it easier to comprehend because there is less code and the conditions are simpler.

1. Separate isFirstLine condition into nested if statement.
2. Pull up negation from !==. These two steps prepare the removal of the redundant else-if condition.
3. Remove redundant condition on else-if because it is always true. After removing the redundant else-if condition, it becomes clear that the final return statement is unreachable.
4. Remove unreachable code. Unreachable code is useless and consumes some of our attention without benefit. It is almost always better to remove it.
5. Push negation back into ===. This refactoring reverts a previous step that was temporarily necessary to enable further refactorings.
6. Invert !== condition and merge nested if. The resulting line === "" condition is easier to understand because there is no negation. Even better, it enables lifting the inner if statement into an else-if sequence and indicates that the empty line handling might be a special case.

After applying these steps, the function looks as follows:

const lineChecker = (line, isFirstLine) => {
  if (line === "") {
    return `<br />`;
  } else if (isFirstLine) {
    return `<h1>${line}</h1>`;
  } else {
    return `<p>${line}</p>`;
  }

};

Improve Readability

The last set of refactorings aims to improve the readability by moving the special case line === '' into a guard clause and using a conditional return expression.

1. Convert line === '' condition into guard clause.
2. Simplify return with conditional expression.
3. Format, e.g., with Prettier on save.

Here is the final result:

const lineChecker = (line, isFirstLine) => {
  if (line === "") {
    return `<br />`
  }

  return isFirstLine ? `<h1>${line}</h1>` : `<p>${line}</p>`;
};

Additional Considerations

This blog post shows how to use automated refactorings to simplify a JavaScript function without changing its behavior. In practice, there are many additional considerations:

• Automated Test Coverage
  Automated testing is essential to have confidence that the refactoring did not inadvertently change the code's behavior. It is particularly crucial when there are error-prone manual refactoring steps. When there is insufficient test coverage, it is critical to add tests before refactoring code.

• Uncovering Potential Bugs
  Simpler code can uncover potential bugs that you can investigate after the refactoring is completed. In the example here, a <br /> is being returned from the function even when isFirstLine is true, which might not be the intended behavior.

• Other Refactorings
  There are many ways to refactor the function from this blog post. I've focussed on simplifying the function, but renaming or even decomposing it are other possibilities. Check out the post "How would you refactor this JS function?" for more.

I hope this post gave you an idea of how to sequence automated refactoring steps to achieve a more significant refactoring change.

Comments

[𒎏Wii 🏳️‍⚧️]

As always, I would much prefer the form

const lineChecker = (line, isFirstLine) => {
   if (line === "")
      return `<br />`
   else if (isFirstLine)
      return `<h1>${line}</h1>`
   else
      return `<p>${line}</p>`
}

Or alternatively

const lineChecker = (line, isFirstLine) =>
   (line === "")
      ? `<br>`
      : isFirstLine
         ? `<h1>${line}</h1>`
         : `<p>${line}</p>`

Early return for anything other than errors just makes the code harder to follow and should be avoided whenever it's not really necessary, which is often a sign that a function does too much.