i am making a project in Nodejs and express i have a logic in response to a ajax call which query MongoDb database via its aggregate framework and then process the result so obtained.

Result obtained from MongoDB Query

  [ { _id: 'ALR', count: 7 },
  { _id: 'WTK', count: 3 },
  { _id: 'BWL', count: 9 },
  { _id: 'BAT', count: 9 } ]

Callback function for this query

, function (err, counts) {
              if(err){
                console.log(err);
                return res.json({status:false, msg : err});
              }
              if(counts){
                console.log(counts);
                for (var i = 0; i < counts.length; i++) {
                    var type = counts[i]._id;
                    var count = counts[i].count;
                    if(type == "ALR"){
                      if(count < 5){
                        return res.json({status:false, msg : "Minimum 5 All-Rounders"});
                      } 
                    } else if(type == "BAT"){
                      if(count < 6){
                        return res.json({status:false, msg : "Minimum 6 Batsman"});
                      } 
                    } else if (type == "BWL"){
                      if(count < 6){
                        return res.json({status:false, msg : "Minimum 6 Bowlers"});
                      } 
                    } else if(type == "WTK"){
                      if(count < 3){
                        return res.json({status:false, msg : "Minimum 3 Wicket Keepers"});
                      } 
                    }
                } 
                return res.json({status:true, msg : "Squad Lauched"});
              }
          });

looks like i am in a kind of If Else hell , this code work as expected but i am not satisfied with this if anyone can atleast give a hint of doing this callback function in a better way.

i am making a project in Nodejs and express i have a logic in response to a ajax call which query MongoDb database via its aggregate framework and then process the result so obtained.

Result obtained from MongoDB Query

  [ { _id: 'ALR', count: 7 },
  { _id: 'WTK', count: 3 },
  { _id: 'BWL', count: 9 },
  { _id: 'BAT', count: 9 } ]

Callback function for this query

, function (err, counts) {
              if(err){
                console.log(err);
                return res.json({status:false, msg : err});
              }
              if(counts){
                console.log(counts);
                for (var i = 0; i < counts.length; i++) {
                    var type = counts[i]._id;
                    var count = counts[i].count;
                    if(type == "ALR"){
                      if(count < 5){
                        return res.json({status:false, msg : "Minimum 5 All-Rounders"});
                      } 
                    } else if(type == "BAT"){
                      if(count < 6){
                        return res.json({status:false, msg : "Minimum 6 Batsman"});
                      } 
                    } else if (type == "BWL"){
                      if(count < 6){
                        return res.json({status:false, msg : "Minimum 6 Bowlers"});
                      } 
                    } else if(type == "WTK"){
                      if(count < 3){
                        return res.json({status:false, msg : "Minimum 3 Wicket Keepers"});
                      } 
                    }
                } 
                return res.json({status:true, msg : "Squad Lauched"});
              }
          });

looks like i am in a kind of If Else hell , this code work as expected but i am not satisfied with this if anyone can atleast give a hint of doing this callback function in a better way.

• javascript
• node.js
• express

Share Improve this question Follow asked Feb 27, 2018 at 9:01 Gaurav KumarGaurav Kumar 71811 gold badge88 silver badges1616 bronze badges 0

Add a ment  | 

5 Answers 5

Sorted by: Reset to default 9

Your code is obviously very repetitive. One approach to change that is to create an object with the necessary data and do the program logic based on that. For example:

var types = {
  ALR: { min: 5, name: 'All-Rounders' },
  BAT: { min: 5, name: 'Batsman' },
  BWL: { min: 4, name: 'Bowlers' },
  WTK: { min: 3, name: 'Wicket Keepers' },
};

for (var i = 0; i < counts.length; i++) {
  var type = counts[i]._id;
  var count = counts[i].count;
  if (types[type] && count < types[type].min) {
    return res.json({ status: false, msg: 'Minimum ' + types[type].min + ' ' + types[type].name });
  }
}

Because you have chosen to return from all of your paths which identify a failure with the data then you don't need to use most of those else statements - simply continue to a subsequent if.

And you are performing two tests for each if which can be concatenated into a single check with 2 conditions and a boolean &&.

And your code doesn't handle the condition when there is no data. Given your approach of multiple returns, you can put that first and remove another level of nesting.

function (err, counts) {
    if(err){
        console.log(err);
        return res.json({status:false, msg : err});
    }

    if(!counts) {
        // only guessing at what you'd want to return here
        return res.json({status:false, msg : "There was no squad!"});
    }

    console.log(counts);
    for (var i = 0; i < counts.length; i++) {
        var type = counts[i]._id;
        var count = counts[i].count;

        if(type == "ALR" && count < 5){
            return res.json({status:false, msg : "Minimum 5 All-Rounders"});
        } 

        if(type == "BAT" && count < 6){
            return res.json({status:false, msg : "Minimum 6 Batsman"});
        } 

        if (type == "BWL" && count < 6){
            return res.json({status:false, msg : "Minimum 6 Bowlers"});
        } 

        if(type == "WTK" && count < 3){
            return res.json({status:false, msg : "Minimum 3 Wicket Keepers"});
        } 
    } 

    return res.json({status:true, msg : "Squad Lauched"});
}

Except for a switch case there isn't much you can do.

However you can cut a bit of code like so:

When you check for error if (err) return; the code-block will the stop executing. So you don't have to check for the if (counts). Instead, just continue writing code.

Example:

function (err, counts) {
              if(err){
                console.log(err);
                return res.json({status:false, msg : err});
              }
              if(counts){ //remove this if. Not needed
                console.log(counts);
                for (var i = 0; i < counts.length; i++) {

Bees

  function (err, counts) {
                  if(err){
                    console.log(err);
                    return res.json({status:false, msg : err});
                  }

                    console.log(counts);
                    for (var i = 0; i < counts.length; i++) 

The same could be done with that inside the for loop, also putting the nested statements together with their parent, like this:

for (var i = 0; i < counts.length; i++) {
    var type = counts[i]._id;
    var count = counts[i].count;
    if(type == "ALR" && count < 5){
        return res.json({status:false, msg : "Minimum 5 All-Rounders"});
    } 
    if(type == "BAT" && count < 6){
        return res.json({status:false, msg : "Minimum 6 Batsman"});
    } 
    if (type == "BWL" && count < 6){
        return res.json({status:false, msg : "Minimum 6 Bowlers"});
    }
    if(type == "WTK" && count < 3){
        return res.json({status:false, msg : "Minimum 3 Wicket Keepers"});
    } 
} 

I would solve it like this:

const squadTypeResolvers = new Map();
squadTypeResolvers.set('ALR', { 
  name: 'All-Rounders',
  minCount: 3,
});
squadTypeResolvers.set('BAT', { 
  name: 'Batsman',
  minCount: 6,
});
squadTypeResolvers.set('BWL', { 
  name: 'Bowlers',
  minCount: 6,
});
squadTypeResolvers.set('WTK', { 
  name: 'Wicket Keepers',
  minCount: 3,
});

function handleErrors(err, counts) {
  if(err){
    return res.json({status:false, msg : err});
  }
  if(counts){
    const errorMessages = [];
    counts.forEach((count) => {
      const squadMember = squadTypeResolvers.get(count._id);
      if (count.count < squadMember.minCount) {
        errorMessages.push('Minimum ' + squadMember.minCount + ' ' + squadMember.name);
      }
    });

    if (errorMessages.length > 0) {
      return res.json({
        status: false,
        msg: errorMessages.join('; '),
      });
    } else {
      return res.json({status:true, msg : "Squad Lauched"});
    }
  }
}

Alternatively you can pass the array of error messages as an array in json to pass all errors conveniently to the user/consumer

Simple. You can use switch case instead of multiple if else statements.