Chaining logical operators gives wrong result in javaScript

Question

I'm trying to apply a medical score in a web application. The search function didn't provide me satisfactory results.
A part of the score is not mathematically but mere logically determined. It needs three variables which values are inspected and scored.

Scoring works like this:

Score        Grades
0    <--     All 0
1    <--     At least one 1, but no >1
2    <--     2 in only one region       (respectively the variable)
3    <--     2 in more than one region  (respectively the variable)
4    <--     3 in one or more region    (respectively the variable)
5    <--     4 in only one region       (respectively the variable)
6    <--     4 in more than one region  (respectively the variable)

Last year I developed a Python script that was functional but I wanted to implement this score via javaScript into a web app. I came up with a counting function - to count how many times a certain number appears.

arr = [];
var a = 2;
var b = 4;
var c = 0;
arr.push(a, b, c);
function countThis(numberToCount) {
    count = 0;
    for (var i in arr) {
        if (arr[i] == numberToCount) {
            count++;
        }
    }
}

These lines seem to work. But now comes the part that bugs me.
I set up a bunch of conditions - all of them chained and combined with the function above - to calculate the score.

function scoreArr() {
    score = 0;
    if (a === 0 && b === 0 && c === 0) {
        score = 0;
    } else if (a <= 1 && b <= 1 && c <= 1 && countThis(1) >= 1) {
        score = 1;
    } else if (a <= 4 || b <= 4 || c <= 4 && countThis(4) >= 2) {
        score = 6;
    } else if (a <= 4 || b <= 4 || c <= 4 && countThis(4) == 1) {
        score = 5;
    } else if (a <= 3 || b <= 3 || c <= 3 && countThis(3) >= 1) {
        score = 4;
    } else if (a <= 2 || b <= 2 || c <= 2 && countThis(2) > 1) {
        score = 3;
    } else if (a <= 2 || b <= 2 || c <= 2 && countThis(2) == 1) {
        score = 2;
    }
    return score;
}

a = 0, b = 0, c = 0 gives the correct score of 0. But with every other possible combination (numbers 0 to 4) I get the exact same result: 6. Which is clearly wrong.

I tried to change the operators, added parenthesis, ... but nothing made it work. I think that the problem is hidden (?) in my conditions, but I don't know how to fix it.

Here is the JSFIDDLE.

I must admit that I'm pretty new to coding javaScript. So if anybody can explain to me what I did wrong, I would be very grateful.

Thank you very much for your time.

countThis doesn't return anything...

doldt
– doldt

2015-07-17 09:29:00 +00:00
Commented Jul 17, 2015 at 9:29 — doldt
– doldt, Commented Jul 17, 2015 at 9:29

Daniel Causebrook · Accepted Answer · 2015-07-17 10:02:11Z

1

I believe one of your problems is that your function countThis(numberToCount) has no return statement, so is returning undefined.

Simply adding return count; should fix things.

function countThis(numberToCount) {
    var count = 0;
    for (var i in arr) {
        if (arr[i] == numberToCount) {
            count++;
        }
    }
    return count;
}

It would also make your code clearer and less prone to errors if you did your if block the other way round, checking from 6-0, requiring one comparison per if statement. This was where you were going wrong before, and just a little bit of rearrangement and planning can help:

function scoreArr() {
    var score = 0;
    if (countThis(4)>1) {
        score = 6;
    } else if (countThis(4)==1) {
        score = 5;
    } else if (countThis(3)>=1) {
        score = 4;
    } else if (countThis(2)>1) {
        score = 3;
    } else if (countThis(2)==1) {
        score = 2;
    } else if (countThis(1)>=1) {
        score = 1;
    } else {
        score = 0;
    }
    return score;
}

I made a jsFiddle so you can see the result: https://jsfiddle.net/Daniel300/evLqzuvs/5/

EDIT:

Having taken another look at your code, I realised there are a couple of other issues that you might want to look at:

If you want your functions to work correctly, you should declare all variables you are using inside the function, and just return what you need.

Instead of:

addOne(1);
function addOne(num) {
  result=num+1;
}
alert(result);

Try:

function addOne(num) {
  var result = num + 1; //Local variable, keeps it inside the function only.
  return result; //Returns only what you need
}
alert(addOne(1)); //MUCH simpler :)

//alert(result); would give undefined

edited Jul 17, 2015 at 10:02

answered Jul 17, 2015 at 9:42

Daniel Causebrook

4771 gold badge8 silver badges21 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

MrBlackhawk Over a year ago

That was a great and simple solution! Despite a minor flaw: It should be countThis(3)>=1 not >1. But now I got the hang of it. Thank you very much!

Daniel Causebrook Over a year ago

@MrBlackhawk Ah yes, I missed this. I have updated it now. I also added more information, as I believe there's also something else that might make your code easier to work with.

Harpreet Singh · Accepted Answer · 2015-07-17 09:39:25Z

Possible reason of getting Score 6 is that:

You have added the condition

Say Group 1

} else if (a <= 4 || b <= 4 || c <= 4 && countThis(4) >= 2) {
    score = 6;
} else if (a <= 4 || b <= 4 || c <= 4 && countThis(4) == 1) {
    score = 5;

before this

say Group 2

} else if (a <= 3 || b <= 3 || c <= 3 && countThis(3) >= 1) {
    score = 4;
} else if (a <= 2 || b <= 2 || c <= 2 && countThis(2) > 1) {
    score = 3;
} else if (a <= 2 || b <= 2 || c <= 2 && countThis(2) == 1) {
    score = 2;

No matter how small is the value of a is, the condition at Group 1 will get execute first and Group 2 will never get execute event if a = 1 or 2 or 3 because these all values satisfied the condition a <= 4

To fix it you need to move your conditions as

function scoreArr() {
    score = 0;
    if (a === 0 && b === 0 && c === 0) {
        score = 0;
    } else if (a <= 1 && b <= 1 && c <= 1 && countThis(1) >= 1) {
        score = 1;
    } else if (a <= 2 || b <= 2 || c <= 2 && countThis(2) == 1) {
        score = 2;
    } else if (a <= 2 || b <= 2 || c <= 2 && countThis(2) > 1) {
        score = 3;
    } else if (a <= 3 || b <= 3 || c <= 3 && countThis(3) >= 1) {
        score = 4;
    } else if (a <= 4 || b <= 4 || c <= 4 && countThis(4) == 1) {
        score = 5;
    } else if (a <= 4 || b <= 4 || c <= 4 && countThis(4) >= 2) {
        score = 6;
    }
    return score;
}

Here is your updated Fiddle

Thank you very much for your effort. Sadly, it didn't solve the problem - just shifted it.

Collectives™ on Stack Overflow

Chaining logical operators gives wrong result in javaScript

2 Answers 2

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related