Implementation: Refactoring

Once you set up your code to be refactorable, you may encounter a number of motivations for refactoring. Here are a few.

Factorization

DRY is a common motivation for refactoring, and it can be applied down to code lines level.

Consider for instance the following code:

if (time.hasDayOfMonth()) {
  result.append(timeMessages.on(time.isApproximate), contents)
} else {
  result.append(timeMessages.in(time.isApproximate), contents)
}

Doesn’t seem very complex, but it still contains some repetitions, which impact maintainability and possibly reliability:

• you need to carefully read the if + else lines to find where is the difference (the on() versus the in());
• all the duplicated parts need to be updated when you want to do something different than append() or provide different parameters to on() and in(). If you forget to update one of them (or don’t update it consistently), you may end up with inconsistent behavior/bugs.

To avoid this, focus on the difference, then inject that resolved difference into the common code:

const message = time.getDayOfMonth() ? timeMessages.on : timeMessages.in
result.append(message(time.approximate), replacement)

This also improves the readability by reducing the number of lines, and making obvious that this code will simply append a message in any case.

Invariants

Sometimes you’ll find that something is constant, not influenced by the calling context, that is, parameters:

class MyClass {

  constructor(value) {
    this.value = value
  }

  method(params) {
    const thing = new Thing(this.value)  // Never depends on params
    use(thing, params)
  }
}

Each method() call above will create a fresh instance, which (unless having a new instance has a side effect) means that this thing is part of the invariant of the object class. As a result, the code can then be refactored as:

class MyClass {
  /**
   * @readonly
   */
  thing

  constructor(value) {
    this.thing = new Thing(value)
  }

  method(params) {
    use(this.thing, params)
  }
}

This way you will:

• simplify code;
• improve performance, by avoiding unnecessary re-instantiation;
• improve reliability by ensuring the invariant part cannot vary (i.e. is read-only).

Function extraction

It is quite common in “long” code blocks to see comments added to clarify what a group of lines does:

...
// This code fetch a user and initializes its messages
user = fetchUser(userKey)
messages = fetchMessages(user.locale)
userReady(user)
...

Now think a minute about the reason why you commented those lines inside the block. Obviously because this wan’t clear enough. Also because those lines formed a logical group of instruction that could be summarized with your comment.

So, why isn’t this group a dedicated function? Each time you do this, refactor the block as a function:

/**
 * This code fetch a user and initializes its messages
 */
loadUser(userKey) {
  user = fetchUser(userKey)
  messages = fetchMessages(user.locale)
  userReady(user)
  return user
}

...
user = loadUser(userKey)
...

You comment will become a call of a function whose name could be exactly what you put in the comment. If too long, this could be the documentation of that function. This way, you’ll improve drastically the readability of your code by both adding semantics and documentation to a group of lines and reducing the size of the initial block. And don’t worry about performance, the VM can inline those things.

Resource management

Another common case is error management when acquiring and releasing resources.

Conclusion

• Don’t repeat yourself;
• Don’t create multiple times things that doesn’t change;
• Anytime you add a comment in a block, consider extracting the commented lines as a function.
• Always make sure that an acquired resource is released, whatever the errors that may occur.