Changing value of a variable within an if statement in Javascript

Question

I'm having a problem with a piece of code and it's driving me nuts. I've been stuck on this for hours, and the worst part is that I'm assuming it's really simple; I just can't figure it out.

I'm trying to make a simple event calendar using Javascript/jQuery. This is the simplified code that I have:

var currentMonth = 1;
if (currentMonth == 1) {
    $("#prev-month").click( function() {
        currentMonth = 12;
    });
    $("#next-month").click( function() {
        currentMonth = 2;
    });
}
if ( currentMonth == 2) {
    $("#prev-month").click( function() {
        currentMonth = 1;
    });
    $("#next-month").click( function() {
        currentMonth = 3;
    });
}
if ( currentMonth == 3) {
    $("#prev-month").click( function() {
        currentMonth = 2;
    });
    $("#next-month").click( function() {
        currentMonth = 4;
    });
}
if ( currentMonth == 4) {
    $("#prev-month").click( function() {
        currentMonth = 3;
    });
    $("#next-month").click( function() {
        currentMonth = 5;
    });
}

Now, every time I click the button with the ID "next-month", it is always 2. If I click the button with the ID "prev-month", it is always 12. It never changes. What am I doing wrong?

Mark Byers · Accepted Answer · 2012-03-11 20:24:51Z

6

Your script code is only run once. You aren't changing the click handlers after you clicked them, so those handlers will always give the same results.

But instead of fixing that it would be simpler to use a single handler for each button and using arithmetic to calculate the next/previous month instead of having 12 handlers which you change at runtime.

$("#prev-month").click( function() {
    currentMonth = (currentMonth - 1) || 12;
});
$("#next-month").click( function() {
    currentMonth = currentMonth % 12 + 1;
});

edited Mar 11, 2012 at 20:24

answered Mar 11, 2012 at 19:54

Mark Byers

843k202 gold badges1.6k silver badges1.5k bronze badges

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

6 Comments

Mark Byers Over a year ago

Yeah, thanks. I thin it's fixed now, though I don't particularly like the + 10. The 10 is a "magic number" that seems to appear from nowhere. Probably best to write out the code more explicitly like Neysor's answer.

Jared Farrish Over a year ago

Personally, I would use one handler and a closure scope for the variable, instead of the (implied) global scope. If it needs to be used externally, $(this).parent().data('data-current-month', currentMonth).

user1106925 Over a year ago

Instead of the magic number, you could do (currentMonth - 1) % 12 || 12;

user1106925 Over a year ago

...or actually just this... (currentMonth - 1) || 12;

Mark Byers Over a year ago

@amnotiam: That's nice! Thanks.

|

Neysor · Accepted Answer · 2012-03-11 19:56:02Z

3

you are using the .click() function wrong. You should make it like this:

var currentMonth = 1;

$("#prev-month").click(function() {
    currentMonth--;
    if (currentMonth == 0) {
        currentMonth = 12;
    }
}
$("#next-month").click(function() {
    currentMonth++
    if (currentMonth == 13) {
        currentMonth = 1;
    }
});

answered Mar 11, 2012 at 19:56

Neysor

3,91511 gold badges36 silver badges67 bronze badges

1 Comment

Ryan Brenner Over a year ago

Thanks for this code as well. I didn't realize that I had to put the statement inside of the function.

Jared Farrish · Accepted Answer · 2012-03-11 20:02:59Z

You can use a closure to store your reference and only one click handler (using $(this).is()):

<div>
    Current Month: <input type="text" id="current-month"/>
    <button id="prev-month">Previous Month</button>
    <button id="next-month">Next Month</button>
</div>

$(document).ready(function(){
    var currentMonth = 1,
        $currentmonth = $('#current-month');

    $currentmonth.val(currentMonth);

    $("#prev-month, #next-month").click(function() {
        var $this = $(this);

        if ($this.is('#prev-month')) {
            currentMonth = currentMonth - 1;
        } else {
            currentMonth = currentMonth + 1;
        }

        if (currentMonth == 0) {
            currentMonth = 12;
        } else if (currentMonth > 12) {
            currentMonth = 1;
        }

        $currentmonth.val(currentMonth);

        console.log('Current Month: ' + currentMonth);
    });
});

http://jsfiddle.net/pCs2G/

Collectives™ on Stack Overflow

Changing value of a variable within an if statement in Javascript

3 Answers 3

6 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

6 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related