NOT ONLY JavaScript Coding & Best Practices Style Guide

Jan 6, 2016·7 min read

During my daily JavaScript development I see a lot of code written by myself and other people that should be better written in a different style. Here I collect some of my personal suggestions to improve these code snippets.

Naming of variables, functions, …

What

Name variables, functions etc. as specific as possible.

Why

This improves the readability of your code for you and other people.

Bad

function find(id) {
...
}

Good

function findCustomer(customerId) {
...
}

Conditions for if, while, …

What

Create boolean variables to name conditions in if, while, ….

Why

This improves the readability of your code for you and other people.

Bad

if (index > 0 && index < maxLen && person.age > 18) {
...
}

Good

var isPersonAdult = person.age > 18;
  var isIndexValid = index > 0 && index < maxLen;
  if (isIndexValid && isPersonAdult) {
...
}

Check parameters

What

Because JavaScript allows you to call a function with any parameter, it seems to be a good practice to check these.

Why

Ignoring or simply returning hides errors.

Bad

function addCustomer(customer) {
    if(!customer) {
      return;
    }
...
}

Good

function addCustomer(customer) {
    console.assert(!!customer, 'customer not given');
    console.assert(customer instance of Customer, 'no costumer object given');
...
}

Comment where a comment is needed

What

Don’t bore with comments that are obvious, instead comment the not so obvious things.

Why

Boring demotivates and secrets decreses consciousness power. Always ask yourself: Is this really obvious or could it be misunderstood?

Bad

//sets customerId to '4'
var id = '4';
...
scope.tabIndex = (value) ? "-1" : "0";

Good

var customerId = '4';
...
//ignore control during tabbing if disabled (-1), or handle 
//tabbing in source order (0) otherwise
scope.tabIndex = (disabled) ? "-1" : "0";

Provide variables with default values.

What

Sometimes I find code that has to check if a variable is defined and if not it is initialised with i.e. an empty array.

Why

You have to write additonal code to check the variable each time you want to use it.

Bad

var selectedCustomerIds;
...
if (!selectedCustomerIds) {
  selectedCustomerIds = [];
}
selectedCustomerIds.forEach(...);

Good

var selectedCustomerIds = [];
...
selectedCustomerIds.forEach(...);

Replace switch case by a dictionary

What

If you are simply replacing one constant value through another constant value, it is easier to use a object for that.

Why

Makes your code simpler and faster.

Bad

var lockFlagText;switch (lockFlag) {
    case 'l':
        lockFlagText = 'Lost';
        break;    case 'd':
        lockFlagText = 'Defect';
        break;    case 'f':
        lockFlagText = 'Fired';
        break;    case ' ':
        lockFlagText = 'Not locked';
        break;
}

Good

var LOCK_FLAG_TEXTS = {'l': 'Lost', 'd': 'Defect', f: 'Fired', ' ': 'Not locked'};
  ...
var lockFlagText = LOCK_FLAG_TEXTS[lockFlag];

Avoid anonymous functions

What

It is a common practice to use anonymous function declarations for defining a callback or something else. Try to avoid this and instead name each function explicitly.

Why

During debugging it is annoying to read anonymous function, because that does not help to understand what is going on.

You also had to name it, if you want to programm a recursive function, calling itself inside.

Bad

var controller.setEventHandler( function () {
...
}

Good

var controller.setEventHandler( function handleCustomerIdChangedEvent() {
...  
}

Crash early

What

It’s always a bad idea to silently catch exceptions or not executing a function when a given parameter is missing. You should have good arguments when doing so.

Why

Simply because you are hiding errors in using the function or executing code. This does not help to get your program free of errors and even worse hides the source of the problem poping up somewhere else.

Bad

function saveCustomer(customerEntity)  {
    if(!customerEntity) {
       return;
    }    try {
      resource.update(customerEntity);
    }
    catch (e) {
      //error occured
    }
  }

Good

function saveCustomer(customerEntity)  {
    console.assert(customerEntity, 'customerEntity not given.');
    ...
    try {
      resource.update(customerEntity);
    }
    catch (exception) {
      console.log(exception); //or create business exception or ...
      throw exception;
    }
  }

Use constants

Use properly named constants instead magic values spreaded all over the code.

Why

The code is more readable for you and others and also robust to changes, because the change happens at exactly one point in the source.

Bad

...
var tax = price * 0.19;

Good

var TAX_RATE = 0.19; //19%
...
var tax = price * TAX_RATE;

And even better with ECMAScript 2015 use const ;-)

Avoid extending build in types

I recently extended an array to contain additional data. Why? Because it is possible and I was to lazy to move it to an object containing an array with the additional data.

But don`t do that or at least keep in mind what could happen.

Why

Everybody else is expecting a simple array, and so maybe does not see the additonal attribute. So does angular.equals ;-)

Bad

var customerIds = [ 1, 2, 3, 4 ];
customerIds.lastSelectedId = 4;

Good

var customerSelection = { 
  lastSelectedId: 4, 
  ids: [ 1, 2, 3, 4 ]
};Example?

Avoid parameter hell

To much parameters given to a function or method makes the code unreadable or even difficult to read. User parameter objects to avoid that.

Why

First the code is more readable, because you are naming the parameters where they are created and used for the call. Even better, the signature of the function does not change over time you add new parameters. Old calls still work, no change needed.

Bad

function calculatePerformance(isAdult, hasChildren, alive, worksHard, knowsEverything, countsAlot, moneyToSpend, dayOfSelection, isImportant) {
...
}...
calculatePerformance( true, false, true, true, false, true, 1234.56, day, true);

Good

function calculatePerformance(dayOfSelection, moneyToSpend, options) {
...
}var options = {
  isAdult: true,
  hasChildren: false, 
  alive: true, 
  worksHard: true, 
  knowsEverything: false, 
  countsAlot: true, 
  isImportant: true 
};calculatePerformance(day, 1234.56, options);

Use parentheses

Even if you know the exact operator precedence, always use parantheses to explicit define the order of evaluation a statement.

Why

For the reader of your code it is annoying to think about the evaluation order. Also if you do use parentheses, there is no reasoning, whether you missed the parantheses or not.

Bad

var a = 4 * 3 + 2;

Good

var a = (4 * 3) + 2;

Don’t create mammoth functions?

Today I discovered a function that was able to do multiple actions for various input constellations. It uses ‘if then else’ constructs to distinguish the purpose. I was wondering, why the author did not add more specific functions, that only does one thing, and so could be easy and clear named.

Why

Those functions are hard to read an maintain. Be as concrete as possible to make the code read as a novel, explaning what is going on. If you can’t name the exact purpose of a function you probabily mixing things up, that does not belong together.

Bad

function iCanDoEverything(type, data) {
  if (type === 1) {
    //iDoThis
    ...
  } else if (type === 2) {
    //iDoThat
    ...
  } ...
}

Good

function iDoThis(data) {
...
}function iDoThat(otherData) {
...
}

Use simple function parameters

Sometimes we are lazy and put big objects into the signature of a function to avoid more than one parameter or we do not see the general purpose of the function. That’s most of the time not a good choice.

Why

It makes the function useful only in a few cases where you hold this object in your hands.
It increases dependencies and therefore does not welcome changes.
It can not be used in different scenarios with different objects.

Bad

function calculateAge(customer) {
   var today = Date.now();
   var birthday = customer.dateOfBirth;   // return something like:
   // today - birthday;
   ...
}var aCustomer = {
   id: '4711',
   name: 'Joe Dear',
   dateOfBirth: new Date(...)
};console.log(calculateAge(aCustomer));

Good

function calculateAge(dateOfBirth) {
  var today = Date.now();
  ...
}var aCustomer = {
   id: '4711',
   name: 'Joe Dear',
   dateOfBirth: new Date(...)
};console.log(calculateAge(aCustomer.dateOfBirth));

Use parentheses more often

Every now and than I trap into the pitfall not to use parantheses in calculations and the like. This sometimes lead to hard to find errors, that simply can be avoided by always using parantheses.

Why

The reader and author has not to imagine what is expected here and does not have to know the operand precedence.

Bad

var a = 4 + true == true ? 42 : 0; //sets a to 0 instead of 46

Good

var a = 4 + (true == true ? 42 : 0); //sets a to 46

What?

Name your object attributes without context unless it makes sense to distinguish them from others on the same level.

Why

Dont bore you collegues with repeating the context that should already be clear.

Bad

rack.rackWidth

Good

rack.width

What?

Make an API consistent and do not handle edge cases in another way the user does not expect. E.g. returning an element instead of an array with this element.

Last time I consumed an API using ODATA I was surprised that it handles the case of returning only one element of metadate different from returning multiple entries. For the first case it returns simply an element, in the latter case it returns an array of elements.

Why

As a consumer of an API it is not so nice to handle all these edge cases with almost no win for both sides and it is easy to overlook those cases, causing errors.

Bad

return result.length === 1 ? result[0] : result

Good

return result

What?

Do not change a container while you are iterating it.

Why

Adding or removing items of a container during an iteration could b a bad idea. What should the interpreter do? Ignore those elements or process them? So to be sure to never change a container during iterating through the same container. Maybe you come never through.

Bad

for (const type of allTypes) {
  if (condition) {
    allTypes.push(42)
  }
}

Good

for (const type of allTypes) {
  if (condition) {
    newTypes.push(42)
  }
}
allTypes = allTypes.concat(newTypes)

Comments, adjustments and extensions are welcome ;-)