Skip to main content

Javascript Refactoring, Part 1

Refactoring code is one of my favorite things. I doubt I’m alone here; terms like “bikeshedding” and “yak shaving” exist for a reason. How does one write code that is clear, simple, and elegant? In this series, I’ll review code snippets and explain how I would refactor them. Let’s begin.

The following code is a helper function used to emit an event to some handler functions. The handlers are asynchronous and execute sequentially, like links in a chain, such that any handler can terminate the execution of the chain. Make sense? Let’s see the code.

 class Context {
   private emit<T>(
       handlers: ((context: this, arg: T, next: () => Promise<void>) => Promise<void>)[],
       arg: T,
       next: () => Promise<void>
   ): Promise<void> {
       const list: ((context: TurnContext, arg: T, next: () => Promise<any>) => Promise<any>)[] = handlers.slice();
 
       const context = this;
 
       function emitNext(i: number): Promise<void> {
           try {
               if (i < list.length) {
                   return Promise.resolve(list[i](context, arg, () => emitNext(i + 1)));
               }
 
               return Promise.resolve(next());
           } catch (err) {
               return Promise.reject(err);
           }
       }
 
       return emitNext(0);
   }
 }

First, we can use the typeof keyword to make the types less repetitive. Next, let’s address the recursive helper. Passing an index parameter into a recursive function operating on a list is clunky. We can use destructuring assignment to convey its semantic meaning better. Also, we can remove the assignment of this.

 class Context {
   private emit<T>(
       handlers: ((context: this, arg: T, next: () => Promise<void>) => Promise<void>)[],
       arg: T,
       next: () => Promise<void>
   ): Promise<void> {
       const runHandlers = ([handler, ...remaining]: typeof handlers): Promise<void> => {
         if (!handler) {
           return next();
         }
 
         return handler(this, arg, () => runHandlers(remaining));
       };
 
       return runHandlers(handlers);
   }
 }

Let’s unpack the changes. First, an arrow function replaces the original recursive helper function declaration. As a result, we no longer have to assign this to a variable outside the function scope. I also removed the explicit Promise wrapping because the types obviate the need.

Finally, I moved the recursive state from a combination mutable variable plus closure entirely to the function parameter list. Semantically, the base case of the recursion is when there is no handler to execute. This code is a more natural translation of the concept. As such, it’s more readable and elegant.

Before we wrap this up, let’s take this refactoring one step further. The pattern of executing a series of functions while propagating some context may sound familiar - it’s the reduce function!

 class Context {
   private emit<T>(
       handlers: ((context: this, arg: T, next: () => Promise<void>) => Promise<void>)[],
       arg: T,
       next: () => Promise<void>
   ): Promise<void> {
       const chain = handlers.reduceRight<
         () => Promise<void>
       >((next, handler) => () => handler(this, arg, next), next);
     
       return chain();
   }
 }

Why are we reducing from right to left?! Think of this code as producing the handler chain. We begin with an initial value of next passed to the final handler in the chain. Invoking this handler makes a Promise. This handler should execute after the second-to-last one, so it must be passed, inside a function, to that handler as the next parameter. Reducing yields a function that produces a Promise, so we invoke it to satisfy the original method signature.

In all honesty, the code from the first refactoring pass is what was merged. There is elegance in identifying patterns and abstractions, but you should know your audience. The final code snippet is clever and concise, but it’s not obvious and would require some explanation as a comment. Part of refactoring code is knowing when enough is enough.