Semicolons matter

js-semicolon

When your code’s intention is ambiguous, your JavaScript engine will attempt to fill in the gaps. If there are multiple potential intentions, your engine will tend to assume you meant to do the right thing. Multi-line statements are supported, omitting semicolons is not, and therefore if the case is ambiguous your engine will assume you meant to write a multi-line statement.

Take the following example:

function Logger() { }

// Log something to the console at a specified level
Logger.prototype.log = function(level) {
  console[level || 'log'].apply(console, [].slice.apply(arguments, 1));
}

// Sugar functions for Logger.log
['info', 'warn', 'error'].forEach(function(level) {
  Logger.prototype[level] = function () {
    return this.log.apply(this, [level].concat(Array.from(arguments)));
  }
});

What will happen when this is run? Did you spot the missing semicolons? Here’s a summary of how v8 sees it (I’ve removed the internals for clarity):

Logger.prototype.log = 
  function() { }['info', 'warn', 'error'].forEach(function() { });

Which is perfectly valid code; it is the rough equivalent of:

function foo() { }
Logger.prototype.log = foo['error'].forEach(function() { });

Be unambiguous with your code. If you’re ending a statement, use a semicolon.

ES6: Break it up

break-it-up

With promisesarrow functions, and functional array chaining available to us natively as of ES6, we now have a myriad of ways to consolidate overly verbose code. The problem is simply that this is too funtoo novel, and we all get carried away, cutting away code that isn’t necessary for compilation but improves readability, maintainability and in some cases testability.

This is an actual snippet of code I wrote for a personal project. The project is an Automated LendingClub Bot utilizing my NodeJS Lending Club API Implementation, and this snippet represents its buy function. This method takes an array of filters as an argument, applies them to a list of new loans, and ultimately purchases a $25.00 note for each.

Bot.prototype.buy = function buy(filters) {
  return this.api.loans.getNewListings().then(loans => {
    return filters.reduce((orders, filter) => {
      loans.filter(filter).forEach(loan => {
        if (!orders.has(loan.id)) {
          orders.set(loan.id, loan.createOrder(25.00));
        }
      });

      return orders;
    }, new Map());
  }).then(orders => this.api.account.submitOrders(Array.from(orders.values())));
};

All of these promisesarrow functions and functional array chains allowed me to write a very compact sliver of code that takes care of many functions:

  1. Fetching a list of available new loans from our API,
  2. Creating a list for each filter containing all the matching loans,
  3. Merging the lists together,
  4. Removing duplicates while maintaining insertion order,
  5. Creating an order for each unique loan, and
  6. Submitting those orders through our API.

However, after a good night’s rest and returning to the code the next morning, I had trouble making modifications. First, it took me a few minutes to remember how the logic was working. Then, I realized I’d have to restructure a good portion of the code to implement a new step to the process.

This is when I realized I had gone too far and it was time to separate out the components by function.

Breaking it up

Let’s walk through the steps we might take to make this method more legible, maintainable and testable.

Step 1. Separate logic by function

A good starting point is to separate the logic by function, and be verbose  about it. Forget about trying to be efficient with our lines of code or performance; we can address those concerns later. Our immediate goal is simply to decouple:

Bot.prototype.buy = function buy(filters) {
  // 1. Fetch a list of available new loans
  this.api.loans.getNewListings().then((loans) => {
    
    // 2. Create a list for each filter containing all the matching loans
    const filteredLists = filters.map(filter => loans.filter(filter));
    
    // 3. Merge the lists together
    const allMatches = [].concat.apply([], filteredLists);
    
    // 4. Remove any duplicates
    const uniquesById = new Map();
    
    allMatches.forEach(loan => {
      if (!uniquesById.has(loan.id)) {
        uniquesById.set(loan.id, loan);
      }
    });
    
    const uniques = Array.from(uniquesById.values());
    
    // 5. Create an order for each unique loan
    const orders = uniques.map(loan => loan.createOrder(25.00));
    
    // 6. Submit the orders
    return this.api.account.submitOrders(orders);
  });
};

Now we’ve made the code fairly verbose, and declared a lot of variables, but each step of the method is running sequentially. We can clearly see where one step ends and the next begins. Legibility has already improved. But we can do better.

Step 2. Create reasonable groupings

We now should be thinking about breaking our logic out into their own methods based on their function. However, just as we don’t want all of our logic in one method, we also don’t want to assign each statement to its own method. We should slim our methods down to the smallest reasonable functionality and no smaller. For example, these steps may be useful on their own:

  • [1. getNewListings] and [6. submitOrders] – These are already separated, as methods of my API implementation.
  • [5. createOrders] returns an array of orders for the array of loans passed in.

But these steps are only marginally useful on their own:

  • [2. filterLists] returns an array of arrays. These arrays are more useful to us if they are flattened, and don’t contain duplicates.
  • [3. flatten] returns an array, however it does not remove duplicates.
  • [4. removeDuplicates] returns an array without duplicates.

These steps can be grouped together to form larger, but more reasonable functions:

  1. union is an established operation in programming that effectively flattens and removes duplicates.
  2. filterLists can return the union of the array of arrays. This way, the output of the method is requires no manipulation to be useful to us.

Now that we’ve identified some useful groups, let’s break our steps our into their own methods:

// Step 1: Fetching new loans is already defined in
// bot.api.loans.getNewListings()

// Steps [2,3,4]: Filter Loans
Bot.filterLoans = function filterLoans(loans, filters) {
  const filteredLoanArrays = filters.map(filter => loans.filter(filter));
  return unionBy(filteredLoanArrays, 'id');
};

// Steps [3,4]: Union By
function unionBy(arrays, keyProperty) {
  const flattened = [].concat.apply([], array);
  const uniqueItems = new Map();
  
  flattened.forEach(item => {
    const key = item[keyProperty] || item;
    
    if (!uniqueItems.has(key)) {
      uniqueItems.set(key, item);
    }
  });
  
  return Array.from(uniqueItems.values());
}

// Step 5: Create Orders
Bot.createOrders = function createOrders(loans, amount) {
  return loans.map(loan => loan.createOrder(amount));
};

// Step 6: Submitting orders is already defined in
// bot.api.account.submitOrders(orders)

Now, our distinct logical groupings are re-usable, easy to read and create great targets for unit testing.

Note that filterLoans and createOrders are defined on the Bot class rather than its prototype. This is because these methods are related to the Bot use-case but are not tied to a specific instance of Bot. These methods are exposed because they may be useful to the user of our API outside the context of the buy method.

Additionally, unionBy has been decoupled from our loan logic. Now, it takes a customizable key property to check the array elements for uniqueness. This method can now be exported from this project’s code base into a more general tool-set to be used across projects. Or, we can re-use existing code from an existing library (such as lodash).

You may notice we’re missing the actual buy function. This brings me to the last step.

Step 3. Rebuild

Now that all of our pieces are separated into components, and not cluttering our buy method, we can rebuild the original method using our broken out pieces:

Bot.prototype.buy = function buy(filters) {
  return this.api.loans.getNewListings()
    .then(loans => Bot.filterLoans(loans, filters))
    .then(loans => Bot.createOrders(loans, 25.00))
    .then(orders => this.api.account.submitOrders(orders));
};

This method should require no inline comments; it lists very clear, discernible steps:

  1. Get the new loan listings via the loans API,
  2. Then filter those loans with the specified filters,
  3. Then create $25.00 orders for those loans,
  4. Then submit those orders via the account API

End Product

Here’s a snapshot of (roughly) how this code looks currently as it sits in my local git repository:

import { unionBy } from 'lodash';
import LCApi from 'lc-api';

function Bot(apiKey, investorId) {
  this.api = new LCApi(apiKey, investorId);
}

Bot.filterLoans = function filterLoans(loans, filters) {
  const filteredLoanArrays = filters.map(filter => loans.filter(filter));
  return unionBy.apply(null, filteredLoanArrays.concat(['id']));
};

Bot.createOrders = function createOrders(loans, amount) {
  return loans.map(loan => loan.createOrder(amount));
};

Bot.prototype.buy = function buy(filters) {
  return this.api.loans.getNewListings()
    .then(loans => Bot.filterLoans(loans, filters))
    .then(loans => Bot.createOrders(loans, 25.00))
    .then(orders => this.api.account.submitOrders(orders));
};

Note that we’re still using promises, arrow functions and functional array chains, but instead of using them to reduce our code footprint, we’re striking a balance between readability and brevity. The result is a much more elegant set of methods that is a pleasure to look at. We also exposed some useful functionality to the users of our API in the process, and that’s priceless.

I love ES6. I am extremely excited about the future of JavaScript development. Promises and arrow functions are very powerful and we’re all having a blast seeing what they’re capable of. Let’s just keep in mind that at some point, readability may suffer from overuse / misuse. I truly believe that maintaining this balance between readability and brevity is an art.