Callback hell in multiple if, else if statements

Question

I have function with data and callback parameters. The data is an object with many attributes and I have to validate these. I wrote multiple if, else if statements to do it, but it seems so disgusting to me.

function(data, callback) {

 if (data.a != 'x') {
   logger.log(...);
   return callback({status: false, code: 'x'});
 } else if (data.b != 'y') {
   logger.log(...);
   return callback({status: false, code: 'y'});
 } else if (data.c != 'z') {
   logger.log(...);
   return callback({status: false, code: 'z'});
 } else if (data.d != 'w') {
   logger.log(...);
   return callback({status: false, code: 'w'});
 }

 //... logic ...
 return callback({status: true});
}

I think It's not the appropriate way to do it.

Answer 1

I would include a set of expected values in my validation routine:

function(data, expected, callback) {
  var checkProp = function(prop) {
    return (data[prop] === expected[prop]);
  };

  for (var p in data) {
    if (data.hasOwnProperty(p)) {
      if (!checkProp(p)) {
        callback({status: false, code: expected[p]});
        return;
      }
    }
  }
  callback({status: true});
}

So, for example, if I had an object:

{ a: 1, b: 2}

I might include an expected parameter like this:

{ a: 3, b: 4 }

Here's a working demo:

function validate(data, expected, callback) {
  var checkProp = function(prop) {
    return (data[prop] === expected[prop]);
  };

  for (var p in data) {
    if (data.hasOwnProperty(p)) {
      if (!checkProp(p)) {
        callback({status: false, code: expected[p]});
        return;
      }
    }
  }
  callback({status: true});
}

var obj = { a: 1, b: 2 },
  expected = { a: 1, b: 4 };
validate(obj, expected, function(res) {
  if (!res.status) {
    console.log('failed with value ' + res.code);
  } else {
    console.log('success');
  }
});

Answer 2

There is no easy way to do this because you're reading from different fields on data, and using inequalities. If you were reading from the same key on data (call it code), you could do the following in idiomatic javascript:

function (data, callback) {
  switch(data.code) {
      case 'x':
          logger.log(...);
          callback({status: false, code: 'x'});
          break;
      case 'y':
          logger.log(...);
          callback({status: false, code: 'y'});
          break;
      case 'z':
          logger.log(...);
          callback({status: false, code: 'z'});
          break;
      case 'w':
          logger.log(...);
          callback({status: false, code: 'w'});
          break;
      default:
          callback({status: true})
   }
}

Note that the switch statement is actually fairly distinct from your original functionality. First of all it is a truthy assertion on value, which results in only one branch every running. E.g. in your original code if data were an object like so:

const data = {a: 'a', b: 'b', c: 'c', d: 'd'} every branch would run, while in the switch implementation only the default branch would run. In either case, I would encourage you to rearchitect your solution for ease of reasoning into a shape that fits the switch.

If you are ok with using ES6 syntax/object destructuring, you could hook up your project with a transpiler like babel, and still use multiple keys like your original implementation, to do the following

function ({a, b, c, d}, callback) {
  if (a != 'x') {
    logger.log(...);
    callback({status: false, code: 'x'});
  } else if (b != 'y') {
    logger.log(...);
    callback({status: false, code: 'y'});
  } else if (c != 'z') {
    logger.log(...);
    callback({status: false, code: 'z'});
  } else if (d != 'w') {
    logger.log(...);
    callback({status: false, code: 'w'});
  }

  //... logic ...
  callback({status: true});
}

With a little bit more preconfiguration you could do something like:

const keys = ['a', 'b', 'c', 'd']
const codes = {'a': 'x', 'b': 'y', 'c': 'z', 'd': 'w'}

Your function could now be:

function(data, callback) {
  keys.map(function(key) {
    if (data[key] != codes[key]) {
      logger.log(...);
      callback({status: false, code: codes[key]});
    }})
    //logic
    callback({status: true})
    }

note you could pass in the codemap / keylist as a function argument if you so wish

Answer 3

Since it seems the code is correct but needs to be pleasing to the eyes, here's something you can try:

function(data, callback) {

  var code;

  if (data.a != 'x') code = 'x';
  else if (data.b != 'y') code = 'y';
  else if (data.c != 'z') code = 'z';
  else if (data.d != 'w') code = 'w';

  if (code) {
    callback({ status: false, code: code });
  } else {
    callback({ status: true });
  }
}

If the comparisons are known beforehand, then:

function(data, callback) {

  var code,

    comparisons = [{
      key: 'a',
      val: 'x'
    }, {
      key: 'b',
      val: 'y'
    }, {
      key: 'c',
      val: 'z'
    }, {
      key: 'd',
      val: 'w'
    }];

  for (each of comparisons) {

    if (data[each.key] != each.val) {
      code = each.val;
      break;
    }
  }

  if (code) {
    callback({ status: false, code: code });
  } else {
    callback({ status: true });
  }
}

Answer 4

One way would be to factor the validation into a separate function:

function failureCode(data) {
    if (data.a != 'x')
        return 'x';
    if (data.b != 'y')
        return 'y';
    if (data.c != 'z')
        return 'z';
}

function (data, callback) {
    var code = failureCode(data);
    if (code) {
        logger.log(...);
        return callback({status: false, code: code});
    }

    //... logic ...
}

Also, don't forget to return from the "failed" branch.

Answer 5

How about this? Since you are actually doing a validation and a "submission" you can just throw the codes and catch the result.

function(data, callback) {
    try {
         if (data.a != 'x') {
             throw 'x';
         }

         if (data.b != 'y') {
             throw 'y';
         }

         // etc

         // logic

         callback({status: true});
    }
    catch(code) {
        callback({status: false, code: code});
    }
}