Odd behavior with else if statement (javascript)

Question

As far as I can tell the code works fine normally but when more information is added or edited then the if else statement starts behaving oddly. It's reading the percent value correctly but it's not returning the correct letter string.

var total = function (){
    var earned = 0;

     for(i = 0; i < Assignments.length; i++){
         earned += parseInt(Assignments[i].earned);
     }
     var possible = 0;

     for(i = 0; i < Assignments.length; i++){
         possible += parseInt(Assignments[i].possible);
     }
     var percent = (Math.floor((earned/possible) * 100));

     console.log(percent);

  //grade letter

      if (percent >= 90){
         grade.innerHTML = '';
         grade.innerHTML = 'A ' + percent + '%';
      } else if (percent <= 89 && 80 >= percent){
         grade.innerHTML = '';
         grade.innerHTML = 'B ' + percent + '%';
      } else if (percent <= 79 && 70 >= percent){
         grade.innerHTML = '';
         grade.innerHTML = 'C ' + percent + '%';
      } else if (percent <= 69 && 60 >= percent){
         grade.innerHTML = '';
         grade.innerHTML = 'D ' + percent + '%';
      } else if(percent <= 59 && 0 >= percent){
         grade.innerHTML = '';
         grade.innerHTML = 'F ' + percent + '%';
      } else {grade.innerHTML = '';}



};

After a few inputs it will return something like this:

B is = 73%? Noo!

I think it might be the else if statements conflicting, but honestly I have no idea why this is behaving this way.

what's conflicting? What is returned that you don't want to? How, in exact way is it going wrong? — Amit Joki, Dec 08 '14 at 09:13
Why don't you use a switch-statement? It's much easier to read than a lot of if/else-blocks. http://www.w3schools.com/js/js_switch.asp — WhiteBr0wnie_24, Dec 08 '14 at 09:14
@WhiteBr0wnie_24 How do you use a switch with number ranges? — JJJ, Dec 08 '14 at 09:16
I agree, you'd better use a switch, that's typically the job of switch. Any if/else workaround would be really disgraceful compared to switch :) — enguerranws, Dec 08 '14 at 09:24
Just using the same expressions as he did, using switch(percent) : http://stackoverflow.com/questions/5619832/switch-on-ranges-of-integers-in-javascript — enguerranws, Dec 08 '14 at 09:27
`switch(percent)` wouldn't work when `percent` is 0 (it has to be `switch(true)`.) And I wouldn't call that "the job of switch" when you hack it that way. There are better ways to test for a number range than else-if chains, but switch is not one of them. — JJJ, Dec 08 '14 at 09:28
@Juhana simply do it by switch(boolean_expression) where boolean expression can be anything that evaluates true or false. That's also something like case (grade > 10 && grade < 20). Look at my answer I posted an interesting link where this can be read. — WhiteBr0wnie_24, Dec 08 '14 at 09:30
@Juhana et. al.: While it's *possible* to use a `switch` for this in JavaScript (http://stackoverflow.com/a/17145931/157247, see #3), it's just a long-winded `else if`. — T.J. Crowder, Dec 08 '14 at 09:31
@Juhana So, what are the " better ways to test for a number range than else-if chains" ? — enguerranws, Dec 08 '14 at 09:34
Seeing the second answer of this question (http://stackoverflow.com/questions/5619832/switch-on-ranges-of-integers-in-javascript), this could make this code a lot more readable and efficient. — enguerranws, Dec 08 '14 at 09:40
@enguerranws T.J.'s answer has one good method (or more generally an object/array which you iterate over). The switch(true) method is exactly the same as else-if chain only with "case" in place of "else if". — JJJ, Dec 08 '14 at 09:57

T.J. Crowder · Answer 1 · 2014-12-08T09:29:46.070

You have your comparisons backward in the else ifs, you're using 70 >= percent rather than percent >= 70 (and so on).

Separately, there's no reason to assign '' to innerHTML if you're about to assign something else to it, and there's no reason to reiterate the upper bound (percent <= 89 and such), because you're using else if, so the percent >= 90 branch will have already been followed. Reiterating them is also a maintenance problem (you'll change one but forget to change the other).

So:

if (percent >= 90) {
    grade.innerHTML = 'A ' + percent + '%';
} else if (percent >= 80) {
    grade.innerHTML = 'B ' + percent + '%';
} else if (percent >= 70) {
    grade.innerHTML = 'C ' + percent + '%';
} else if (percent >= 60) {
    grade.innerHTML = 'D ' + percent + '%';
} else if (percent >= 0) {
    grade.innerHTML = 'F ' + percent + '%';
} else {
    grade.innerHTML = '';
}

Or of course, you can use a map, since your grade boundaries are evenly divisible by 10:

// Somewhere central
var grades = {
    6: 'D',
    7: 'C',
    8: 'B',
    9: 'A',
    10: 'A'
};

// ...then simply:
if (percent >= 0) {
    grade.innerHTML = (grades[Math.floor(percent / 10)] || 'F') + ' ' + percent + '%';
} else {
    grade.innerHTML = "Less than 0?!";
}

var grades = {
    6: 'D',
    7: 'C',
    8: 'B',
    9: 'A',
    10: 'A'
};

function showGrade(percent) {
  var grade;
  if (percent >= 0) {
    grade = (grades[Math.floor(percent / 10)] || 'F') + ' ' + percent + '%';
  } else {
    grade = "Less than 0?!";
  }
  snippet.log(grade);
}

var n;
for (n = 0; n <= 100; ++n) {
  showGrade(n);
}

<!-- Script provides the `snippet` object, see http://meta.stackexchange.com/a/242144/134069 -->
<script src="http://tjcrowder.github.io/simple-snippets-console/snippet.js"></script>

The "map solution" is the best IMHO. – enguerranws Dec 08 '14 at 09:42 — enguerranws, Dec 08 '14 at 09:42

Girish · Answer 2 · 2014-12-08T09:24:53.497

If-Else condition range looking wrong, try this code...

var total = function (){
var earned = 0, possible = 0;
for(i = 0; i < Assignments.length; i++){
     possible += parseInt(Assignments[i].possible);
     earned += parseInt(Assignments[i].earned);
 }
 var percent = (Math.floor((earned/possible) * 100));

 console.log(percent);

  //grade letter

  if (percent >= 90){
     grade.innerHTML = 'A ' + percent + '%';
  } else if (percent >= 80 && percent <= 89){
     grade.innerHTML = 'B ' + percent + '%';
  } else if (percent >= 70 && percent <= 79){
     grade.innerHTML = 'C ' + percent + '%';
  } else if (percent >= 60 && percent <= 69){
     grade.innerHTML = 'D ' + percent + '%';
  } else if(percent >= 0 && percent <= 59){
     grade.innerHTML = 'F ' + percent + '%';
  } else {grade.innerHTML = '';}
};

Although i have optimized some code no need 2 loops there, you can sum earned, possible variable in single loop, and no need to .innerHTML = '' when you assigning value in element.

score 0 · Answer 3 · edited May 23 '17 at 12:21

I found a nice post on efficiency using different methods of comparing things. I still suggest using a switch-block instead of if/else, but it's technically not better than your solution, but as I thinks it nicer to read. Here is a code snippet:

switch (true) {
case (0 <= val &&  val < 1000): /* do something */ break;
case (1000 <= val &&  val < 2000): /* do something */ break;
...
case (29000 <= val &&  val < 30000): /* do something */ break;
}

Taken from this post. There's also an interesting table (in the first answer) Switch statement for greater-than/less-than and different methods of making this work. I know this is actually not a solution for your answer, but it might come in handy if you need to add more if/else and are doing some huge lists with values.

Repeating the boundaries lays this wide open to maintenance bugs. But you can do the same thing *without* repeating the boundaries, in JavaScript `switch` cases are evaluated *in order* and only the first matching one is executed (as with `else if`). — T.J. Crowder, Dec 08 '14 at 09:36

Odd behavior with else if statement (javascript)

3 Answers3