TL;DR The first of a series of post about if-else programming, and how to avoid that. In this post I show you how to remove those pesky “ifs”. It’s all javascript. The war on if has started.

Intro

I’m looking at bad code in our codebase, trying not to judge but also trying to offer a better way. Follow me through the world of code, looking at bad code and good code. The snippets are small and in offensive!

In this post I present to you some code, is it bad what did it do wrong? It’s all about the “ifs”. Do you remember the anti-if campaign? It’s a bit old now but the message is still relevent. https://francescocirillo.com/pages/anti-if-campaign

Should I call this Bad Code, should code feel bad about itself? Below (or to the side) is an example of how small functions can make life better. The first code snippet is real code from the codebase (searchlight). It shows a very common issue in javascript land, does this property exist? does it have a value? and can I do something to it. You may see code like this all the time. At least I do in the searchlight codebase. Most of the time this can be avoided by having defaults and protecting the system at the boundaries, basically stop garbage data from getting in. But look at all the “ifs”

There are other issues with this code.

Do we really need to use “for” loop? what about _.forEach? Lots of “ifs” here some thing is iffy. Are we doing the same thing over and over, can a function help with that?

A better approach

Iterators are great if you don’t need the index number then try forEach Small functions can help reduce the “if” issue No need to check if shortAccountSummaryArray is an array, as lodash will not perform the loop if it does not have an iterator. And why not set a default while we are at it?

Unit Tests

You’ve got them right? In this instance there were already some test cases, so a little refactoring was just fine.

Still Not Happy

The code still smells, even the good code can you see why? If we were doing functional programming, then we just broke the golden rule. Don’t mutate data ( something for another day ). I also just created a function called “map” can’t I come up with a better name? And what about my function toUpperCase? It’s a bit of a lie. It also set’s a default value which is what I want but what about the name toUpperCaseWithDefault. Something to think about.

Before

  for (var i = 0; i < data.shortAccountSummary.length; i++) {

    if (data.shortAccountSummary[i].primaryName) {
      data.shortAccountSummary[i].primaryName = data.shortAccountSummary[i].primaryName.toUpperCase();
    }

    if (data.shortAccountSummary[i].surname) {
      data.shortAccountSummary[i].surname = data.shortAccountSummary[i].surname.toUpperCase();
    }

    if (data.shortAccountSummary[i].title) {
      data.shortAccountSummary[i].title = data.shortAccountSummary[i].title.toUpperCase();
    }

    if (data.shortAccountSummary[i].forename1) {
      data.shortAccountSummary[i].forename1 = data.shortAccountSummary[i].forename1.toUpperCase();
    }

    if (data.shortAccountSummary[i].forename2) {
      data.shortAccountSummary[i].forename2 = data.shortAccountSummary[i].forename2.toUpperCase();
    }
  }

After

  const toUpperCase = (value) => value ? value.toUpperCase() : "";

  function map(shortAccountSummaryArray) {
    _.forEach(shortAccountSummaryArray, function (summary) {
      summary.primaryName = toUpperCase(summary.primaryName);
      summary.surname = toUpperCase(summary.surname);
      summary.title = toUpperCase(summary.title);
      summary.forename1 = toUpperCase(summary.forename1);
      summary.forename2 = toUpperCase(summary.forename2);
    })
  };