keep-code-left.md

All code is in JavaScript, but the technique discussed is applicable to many languages.

50% of "else" statements are unnecessary. Not only that, they also increase bugs and decrease readability and maintainability. A coding technique to counter this is "guard clauses". Using them has the following benefits:

• Thinking in terms of guard clauses instead of if/else puts you in a frame of mind where you are more likely to catch bugs.
• This pattern increases code readability (and thus maintainability)
• It Keeps Code Left, which again increases readability

First off, what are guard clauses? They make sure some code state is as it should be before continuing. Perhaps the most common use is this:

someFunction: function (err, results) {
  if (err) throw err;
  ...

Another example:

someFunction: function (arg) {
  if (typeof arg !== 'expectedType') return;
  ...

But they can be used to do more than validate input.

Note: in JavaScript, some uses of the logical AND is sometimes called a guard pattern. Eg:

if (object && object.subObject && object.subObject.method) {
  object.subObject.method();
}

This is not the guard pattern I'm talking about in this post.

Let's start with a common bug that is easily avoided with the guard pattern:

addToMessages: function(results){
  app.clearMessages();
  if(Array.isArray(results)){
    results.forEach(app.addMessage);
  }
}

The bug: If results is not an array, you probably do not want app.clearMessages() to execute. While this function could be fixed thus:

addToMessages: function(results){
  if(Array.isArray(results)){
    app.clearMessages();
    results.forEach(app.addMessage);
  }
}

In more complex code such bugs can be more subtle. Guard clauses help prevent careless mistakes such as these.

Let's look at a slightly more complex example:

addToRooms: function(messages){
  $('#roomSelect').html('<option value="newRoom">New Room...</option>\
                         <option value="lobby" selected>Lobby</option>');

  if (Array.isArray(messages)) {
    var processedRooms = {};
    messages.forEach(function (message){
      var roomname = message.roomname;

      if (roomname && !processedRooms[roomname]) {
        app.addRoom(roomname);
        processedRooms[roomname] = true;
      }
    });
  }
 
  $('#roomSelelect').val(app.room);
}

The bug: If messages is not an array, you probably do not want this function's 1st and last line to do anything. Yet as-is, they would run. Once again, you could rewrite it to:

addToRooms: function(messages){
  if (Array.isArray(messages)) {
    $('#roomSelect').html('<option value="newRoom">New Room...</option>\
                           <option value="lobby" selected>Lobby</option>');
    var processedRooms = {};
    messages.forEach(function (message){
      var roomname = message.roomname;

      if (roomname && !processedRooms[roomname]) {
        app.addRoom(roomname);
        processedRooms[roomname] = true;
      }
    });
    $('#roomSelelect').val(app.room);
  }
}

But it reads better with a guard clause:

addToRooms: function(messages){
  if (!Array.isArray(messages)) return;

  $('#roomSelect').html('<option value="newRoom">New Room...</option>\
                         <option value="lobby" selected>Lobby</option>');
  var processedRooms = {};
  messages.forEach(function (message){
    var roomname = message.roomname;

    if (roomname && !processedRooms[roomname]) {
      app.addRoom(roomname);
      processedRooms[roomname] = true;
    }
  });
  $('#roomSelelect').val(app.room);
}

Now that we've seen how guard clauses help reduce bugs, let's see how they can increase readability by getting rid of unnecessary else statements (note how this also helps "Keep Code Left"):

Before:

createUser: function(req, res, next) {
  var username = req.body.username.trim();
  var password = req.body.password.trim();
  var teamname = req.body.teamname.trim();
  if (username === '' || teamname === '' || password === '') {
    return res.status(400).send('Username, Password, and Teamname must be present');
  } else {
    User.findOne({username: username}, function(err, user) {
      if (err) {
        return res.status(404).send(err);
      } else if (user) {
        return res.status(400).send('Username exists');
      } else {
        var newUser = new User({
          username: username,
          password: password
        });

        newUser.save(function(err, newUser) {
          if (err) {
            return res.status(404).send(err);
          } else {
            Team.findOne({name: teamname}, function(err, team) {
              if (err) {
                return res.status(404).send(err);
              } else {
                team = team || new Team({name: teamname});

                team.users.push(newUser);
                team.save(function (err) {
                  if (err) {
                    return res.status(404).send(err);
                  } else {
                    res.status(201);
                    sendJWT(newUser, res);
                  }
                });
              }
            });
          }
        });
      }
    });
  }
},

After:

createUser: function(req, res, next) {
  var username = req.body.username.trim();
  var password = req.body.password.trim();
  var teamname = req.body.teamname.trim();
  if (username === '' || teamname === '' || password === '') {
    return res.status(400).send('Username, Password, and Teamname must be present');
  }

  User.findOne({username: username}, function(err, user) {
    if (err) return res.status(404).send(err);
    if (user) return res.status(400).send('Username exists');

    var newUser = new User({
      username: username,
      password: password
    });

    newUser.save(function(err, newUser) {
      if (err) return res.status(404).send(err);

      Team.findOne({name: teamname}, function(err, team) {
        if (err) return res.status(404).send(err);
        team = team || new Team({name: teamname});

        team.users.push(newUser);
        team.save(function (err) {
          if (err) return res.status(404).send(err);

          res.status(201);
          sendJWT(newUser, res);
        });
      });
    });
  });
}

We went from 5 else statements to 0. They were just cluttering up the code. This is a powerful example of how guard clauses can help maintain readability and Keep Code Left. ( :