How to do the Jquery if-else statement better

Question

Is there a way on how I can improve this if else statement below:

function updateStatusCount(){
    var openCount = 0;
    var ongoingCount = 0;
    var atRiskCount = 0;
    var missedCount = 0;
    var closedCount = 0;
    var onHoldCount = 0;
    $('.sr-header-status').each(function(){
        var value = $(this).html().toLowerCase();
        if(value === "open"){
            openCount = openCount + 1;
        } else if (value === "ongoing"){
            ongoingCount = ongoingCount + 1;
        } else if (value === "at risk"){
            atRiskCount = atRiskCount + 1;
        } else if (value === "missed target"){
            missedCount = missedCount + 1;
        } else if (value === "closed"){
            closedCount = closedCount + 1;
        } else if (value === "on hold"){
            onHoldCount = onHoldCount + 1;
        }
    });
    $('.statusCount').empty();
    $('.open.status_filter').find('span.statusCount').html(openCount);
    $('.ongoing.status_filter').find('span.statusCount').html(ongoingCount);
    $('.atrisk.status_filter').find('span.statusCount').html(atRiskCount);
    $('.missedtarget.status_filter').find('span.statusCount').html(missedCount);
    $('.closed.status_filter').find('span.statusCount').html(closedCount);
    $('.onhold.status_filter').find('span.statusCount').html(onHoldCount);
}

I'm thinking something like this

function updateStatusCount(){
    getStatusCount("open");
    getStatusCount("ongoing");
    getStatusCount("on hold");
    getStatusCount("at risk");
    getStatusCount("missed target");
    getStatusCount("closed");
}

function getStatusCount(status){
    var count = 0;
    var statusClass = "."+status.replace(/ /g,'');
    var $statusContainer = $(statusClass).find('span.statusCount');
    $('.sr-header-status').each(function(){
        var value = $(this).html().toLowerCase();
        if(value === status){
            count = count + 1;
        }
    });
    $statusContainer.empty();
    $statusContainer.html(count);
}

but which is better?

Thank you

Using [`switch`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/switch) ..? — Mr. Alien, Feb 19 '14 at 07:55
Can you give some (simple/representative) context for this code? How is it used (ideally posting some -again, simple- HTML *here* in your question, and posting a [JS Fiddle demo](http://jsfiddle.net/)). — David Thomas, Feb 19 '14 at 07:57
@newbie, please check my answer, that does not use `each`. Your `getStatusCount()` can have a single line. BTW I think you should use `text()`, not `html()` in your code — Jose Rui Santos, Feb 19 '14 at 08:55

jfriend00 · Accepted Answer · 2014-02-19T08:15:55.053

You can use an object as a lookup mechanism and let the object lookup handle all the comparisons for you rather than coding each one individuallly:

function updateStatusCount(){
    var counts = {
        "open": 0, "ongoing": 0, "at risk": 0,
        "missed target": 0, "closed": 0, "on hold": 0
    }
    $('.sr-header-status').each(function(){
        counts[$(this).html().toLowerCase()]++; 
    });
    $('.statusCount').empty();
    $('.open.status_filter').find('span.statusCount').html(counts["open"]);
    $('.ongoing.status_filter').find('span.statusCount').html(counts["ongoing"]);
    $('.atrisk.status_filter').find('span.statusCount').html(counts["at risk"]);
    $('.missedtarget.status_filter').find('span.statusCount').html(counts["missed target"]);
    $('.closed.status_filter').find('span.statusCount').html(counts["closed"]);
    $('.onhold.status_filter').find('span.statusCount').html(counts["on hold"]);
}

Or, you could get even a little more clever by generating the class name you're looking for:

function updateStatusCount(){
    var counts = {
        "open": 0, "ongoing": 0, "at risk": 0,
        "missed target": 0, "closed": 0, "on hold": 0
    }
    $('.sr-header-status').each(function() {
        counts[$(this).html().toLowerCase()]++; 
    });
    $('.statusCount').empty();
    for (var prop in counts) {
        $("." + prop.replace(/\s/g, "") + ".status_filter" + " span.statusCount").html(counts[prop]);
    }
}

@Leo Sometimes different people have the same idea, writing their answers at the same time. Calling them "copycats" just because they hit "post answer" later is not justified. — Tomalak, Feb 19 '14 at 08:14
@Leo I think it's not ideal to preempt what the community should do. Let them decide. They're smart enough to decide for themselves what should distract them. — Tomalak, Feb 19 '14 at 08:44
@Leo Its not about "personal". I informed you that you made a rather poor decision there for very objective reasons (two, even: once for calling out other people, once for admittedly trying to influence how other people vote). Just put in more consideration next time you comment, that's all. — Tomalak, Feb 19 '14 at 09:08
Ok, boss, I'll consider my comments next time. In the meantime, can you please (i'm begging you to) stop mentioning me your comments, get off my back and get a life mate? — Leo, Feb 19 '14 at 09:20

Jose Rui Santos · Answer 2 · 2014-02-19T09:00:17.650

If you have a finite use cases, then using a switch statement seems to create more simple and legible code. I have made some code refactoring also.

function updateStatusCount(){
    var openCount = ongoingCount = atRiskCount = missedCount =
        closedCount = onHoldCount = 0;

    $('.sr-header-status').each(function(){
        switch ($(this).html().toLowerCase()) {
          case "open":          ++openCount;    break;
          case "ongoing":       ++ongoingCount; break;
          case "at risk":       ++atRiskCount;  break;
          case "missed target": ++missedCount;  break;
          case "closed":        ++closedCount;  break;
          case "on hold":       ++onHoldCount;
        }
    });
    $('.statusCount').empty(); // only need this line if there is other .statusCount not defined below
    $('.open.status_filter span.statusCount').html(openCount);
    $('.ongoing.status_filter span.statusCount').html(ongoingCount);
    $('.atrisk.status_filter span.statusCount').html(atRiskCount);
    $('.missedtarget.status_filter span.statusCount').html(missedCount);
    $('.closed.status_filter span.statusCount').html(closedCount);
    $('.onhold.status_filter span.statusCount').html(onHoldCount);
}

Solution 2 In jQuery 1.8+ you can define a new selector:

$.expr[":"].containsNoCase = $.expr.createPseudo(function(arg) {
    return function( elem ) {
        return $(elem).text().toLowerCase().indexOf(arg.toLowerCase()) >= 0;
    };
});

or this one, if your jquery is older:

$.expr[':'].containsNoCase  = function(a, i, m) { 
  return $(a).text().toLowerCase().indexOf(m[3].toLowerCase()) >= 0; 
};

and finally your function can be simplified to

function getStatusCount(status){
    var statusClass = "." + status.replace(/ /g,''),
        $statusContainer = $(statusClass).find('span.statusCount');
    $statusContainer.text($('.sr-header-status:containsNoCase("' + status + '")').length);
}

score 0 · Answer 3 · answered Feb 19 '14 at 07:57

0

use switch instead:

   $('.sr-header-status').each(function(){
    var value = $(this).html().toLowerCase();

switch(value){
case "open":
    openCount = openCount + 1;
break;

case "at risk":
    atRiskCount = atRiskCount + 1;
break;

case "missed target":
   missedCount = missedCount + 1;
break;
//and so on
.......
}});

answered Feb 19 '14 at 07:57

Milind Anantwar

81,290
25
94
125

I'm confused. The question seems to be pretty abstract, however, since you added an answer, I wonder, why is it better? Why is a `switch` better that `if...else`? Does it add any performance gains or improves code clarity? I don't get it – Leo Feb 19 '14 at 08:01
This will not make anything easier. – Daniel Feb 19 '14 at 08:05
Thanks, but still, it looks out of context to me. There's no such function to be called twice, furthermore, I can simply assign the return value of the function to a variable and then if-else through the variable – Leo Feb 19 '14 at 08:06

How to do the Jquery if-else statement better

3 Answers3