CODEWITHSUNDEEP
javascripthtml
MarsOne
MarsOne

Reputation: 2186

Javascript increment not working

Well I did not know what exactly would be a good title for this because it is a most peculiar situation or I'm abnormally dumb.

Here's what im trying to do.

Create a simple <meter> tag which is new in HTML5. The main issue is with my javascript. Im trying to increment the value of the meter tag gradually in my javascript. But somehow it doesn't work the way i want.

JavaScript.

for (var i = 0; i <= 10; i++) {
    var a = document.getElementById("mtr1");
    setTimeout(function () {
        console.log(i);
        a.value = i;
    }, 250);
}

I'm trying to increase the value of the meter gradually every 250 ms.This doesn't happen. Instead the meter jumps straight to 10.

What interested me was the value of i that i got in the console. I got instances of 10, instead of 1,2,3...10.

Why does this happen?

FIDDLE

Upvotes: 5

Views: 3727

Answers (9)

pax162
pax162

Reputation: 4735

Nobody used setInterval, so here's my solution ( http://jsfiddle.net/Qh6gb/4/) :

var a = document.getElementById("mtr1");
var i = 0;
var interval = setInterval(function () {
    console.log(i);
    a.value = ++i;
    if (i == 10) {
        clearInterval(interval);
    }
}, 250);

Upvotes: 5

Abdennour TOUMI
Abdennour TOUMI

Reputation: 93193

you can use timeout jquery plugin:. It is easier

However you should calculate your timeout ,

For you ,timeout=250*max=250*10=2500

So

$('meter').timeout(2500);

Demo

Upvotes: 1

aga
aga

Reputation: 29416

It's a JavaScript closures' classic. Here i is an actual reference to the variable, not its copy. After you've iterated through the loop it has the value of 10, that's why all log invocations write 10 to log.
This should work better: 

for (var i = 0; i <= 10; i++) {
    var a = document.getElementById("mtr1");
    setTimeout(function (i) {
        return function() {
            console.log(i);
            a.value = i;
        };
    }(i), 250 * i);
}

Here the most inner i is the setTimeout's callback argument, not the variable which you've declared in the loop body.

Upvotes: 7

plalx
plalx

Reputation: 43718

You should read more about closures in JavaScript. When a variable gets closed over, it's the same exact variable, not a copy. Since setTimeout is asynchronous, the whole loop finishes before any of the functions run, therefore the i variable will be 10 everywhere.

DEMO

function incMtrAsync(max, delay, el) {
    if (el.value++ < max) {
        setTimeout(incMtrAsync.bind(null, max, delay, el), delay);
    }   
}

incMtrAsync(10, 250, document.getElementById("mtr1"));

The above implementation implements the loop using a recursive approach. Everytime inMtrAsync is called, it checks if the value of the meter reached the max value, and if not, registers another timeout with a callback to itself.

If you want to delay the initial increment as well, just wrap the first call in another timeout.

setTimeout(incMtrAsync.bind(null, 10, 250, document.getElementById("mtr1")), 250);

Upvotes: 7

Entity Black
Entity Black

Reputation: 3491

It happens because you called setTimeout, which is "asynchronous". So setTimeout is called 10times but after whole loop is done then it is executed. Therefore, i = 10 in each call...

http://jsfiddle.net/Qh6gb/9/

there is the solution:

var i = 1,
    meter = document.getElementById("mtr1");

function increase() {
    meter.value = i++;
    console.log(i);
    if(i<=10) {
        setTimeout(increase, 250);
    }
}

setTimeout(increase, 250);

Upvotes: 2

Matteo Tassinari
Matteo Tassinari

Reputation: 18584

The problem you describe happens before the asyncronous call to setTimeout in your original version sees a value of 10 for i because that is its value at the moment the callback is executed.

So, this is a problem with the scope of the closure, to make it work you should make it like this:

for (var i = 0; i <= 10; i++) {
    var a = document.getElementById("mtr1");
    (function (i, a) {
        setTimeout(function () {
          console.log(i);
          a.value = i;
        }, 250);
    })(i, a);
}

also, since a is always the same, this should be better:

var a = document.getElementById("mtr1");
for (var i = 0; i <= 10; i++) {
    (function (i) {
        setTimeout(function () {
          console.log(i);
          a.value = i;
        }, 250);
    })(i);
}

If then you want to see the counter "ticking up", this will make it visible:

var a = document.getElementById("mtr1");
for (var i = 0; i <= 10; i++) {
    (function (i) {
        setTimeout(function () {
          console.log(i);
          a.value = i;
        }, 1000 * i);
    })(i);
}

See http://jsfiddle.net/LDt4d/

Upvotes: 3

power_scriptor
power_scriptor

Reputation: 3304

I hope I understand right. Please try and tell me if you got solution.

var count = 0;
function increment(){
    document.getElementById("meter").value = count; 
    count++; 
    if(count ==10) 
        count=0;
}
setInterval(increment, 250);

Please check with jsFiddle

Upvotes: 0

user1800281
user1800281

Reputation: 1

You're creating multiple functions that are all being set off at the same time. Multiply the timer by i for correct delay.

for (var i = 0; i <= 10; i++) {
var a = document.getElementById("mtr1");
setTimeout(function () {
    console.log(i);
    a.value = i;
}, 250 * i);
}

Upvotes: -1

Aycan Yaşıt
Aycan Yaşıt

Reputation: 2104

Run for loop inside the function instead of declaring a closure in every step of the loop.

JSFIDDLE: http://jsfiddle.net/Qh6gb/3/

var a = document.getElementById("mtr1");
setTimeout(function () {      
    for (var i = 0; i < 10; i++) {
        console.log(i);
        a.value = i;
    }
}, 250);

Upvotes: 0

Related Questions