CODEWITHSUNDEEP
javascript
Alasdair Marchant
Alasdair Marchant

Reputation: 33

if statement executes even with false condition

I've written some code which is a basic up/down voting list. https://alasdairjames.github.io/up-down-counter1/

This works absolutely fine, apart from with the last list item. All the other list items up and down vote as they should.

With the last list item, if I 'down' vote it a few times, and then 'up' vote it, even if its counter is lower than its parent prev sibling counter, the if statement somehow still runs.

I've checked through all the code and I can't see where the problem is. 

//Move it up 
$(".testMoveUp").on("click", function(event){
    // select the counter, increase it
    const counter = $(event.target).siblings(".datasetCounter");
    let counterNew = +$(counter).text()+1; 
    $(counter).text(counterNew);
    //select this and previous counters
    var thisCounter = $(event.target).siblings(".datasetCounter").text();
    var prevCounter = $(event.target).parent().prev().children(".datasetCounter").text();
    console.log(thisCounter);
    console.log(prevCounter);
    //move if appropriate
    if ( thisCounter > prevCounter) {
        var parent = $(event.target).parent();
        var prevParent = $(event.target).parent().prev();
        $(parent).insertBefore(prevParent);
    }
});

//Move it down
$(".testMoveDown").on("click", function(event){
    // select the counter, increase it
    const counter = $(event.target).siblings(".datasetCounter");
    let counterNew = $(counter).text()-1; 
    $(counter).text(counterNew);
    //select this and previous counters
    var thisCounter = $(event.target).siblings(".datasetCounter").text();
    var nextCounter = $(event.target).parent().next().children(".datasetCounter").text();
    console.log(thisCounter);
    console.log(nextCounter);
    //move if appropriate
    if ( thisCounter < nextCounter) {
        var parent = $(event.target).parent();
        var nextParent = $(event.target).parent().next();
        $(parent).insertAfter(nextParent);
    }
});

Upvotes: 0

Views: 86

Answers (3)

dquijada
dquijada

Reputation: 1699

The problem is on this two lines:

var thisCounter = $(event.target).siblings(".datasetCounter").text();
var nextCounter = $(event.target).parent().next().children(".datasetCounter").text();

You are getting the text value and comparing it.

The easier way to fix it is just to parse the texts to numbers. Since you are sure they will always be numbers, you can simply add '+' on your comparisons:

if (+thisCounter < +nextCounter) {
    ...
}

Note: remember to add it to both comparisons, not only one

Upvotes: 0

axiac
axiac

Reputation: 72186

if (thisCounter < nextCounter) -- you compare strings here, not numbers.

Use parseInt() to store numeric values in thisCounter and nextCounter:

var thisCounter = parseInt($(event.target).siblings(".datasetCounter").text(), 10);

Upvotes: 1

Niet the Dark Absol
Niet the Dark Absol

Reputation: 324620

String "9" is greater than string "10". Make sure to convert your values to numbers before comparing them.

Upvotes: 0

Related Questions