tags:

views:

223

answers:

3

This is my code, SetOpacity get invoked with wrong values, why?

function SetOpacity(eID, opacity){                  
   eID.style.opacity = opacity / 100;
   eID.style.filter = 'alpha(opacity=' + opacity + ')';
}
function fade(eID, startOpacity, endOpacity){           
    var timer = 0;
    if (startOpacity < endOpacity) { 
       for (var i = startOpacity; i <= endOpacity; i++) {
           setTimeout(function() {SetOpacity(eID, i);}, timer * 30);
           timer++;
        }
    }           
}
+3  A: 

This is a closure issue. By the time you run the function, i is already at endOpacity. This will work, by creating another closure:

function SetOpacityTimeout(eID, opacity, timer){
  setTimeout(function() {SetOpacity(eID, opacity);}, timer * 30);
}

function fade(eID, startOpacity, endOpacity){           
    var timer = 0;
    if (startOpacity < endOpacity) {
       for (var i = startOpacity; i <= endOpacity; i++) {
          SetOpacityTimeout(eID,i,timer);
          timer++;
        }
    }           
}
Kobi
I still get wrong results
ronik
Have you tested this? `var opacity` is still in the same scope as `i`, so it should still break, from what I can tell.
Shtééf
@Shtééf - you are correct. working on that.
Kobi
why it works inside external function?
ronik
+1  A: 

Kobi has the right idea on the problem. I suggest you use an interval instead, though.

Here's an example: (Your SetOpacity function remains the same, I left it out here.)

function fade(eID, startOpacity, endOpacity){
    var opacity = startOpacity;
    SetOpacity(eID, opacity);

    var interval = window.setInterval(function(){
        opacity++;
        SetOpacity(eID, opacity);

        // Stop the interval when done
        if (opacity === endOpacity)
            window.clearInterval(interval);
    }, 30);
}
Shtééf
thanks, but how to do it with setTimeout?
ronik
MBO's solution will also work.
Shtééf
+4  A: 

This should work:

for (var i = startOpacity; i <= endOpacity; i++) {
    (function(opacity) {
        setTimeout(function() {SetOpacity(eID, opacity);}, timer * 30);
    })(i);
    timer++;
}

This works as follows:

  • inside loop you create anonymous function (function(...){...}) and immediately call it with parameter (that's why there are parenthesis around function(){}, so you can call it adding () at end and pass parameters)
  • parameters passed to this anonymous function (i value, which is opacity inside function) are local to this anonymous function, so they don't change at next steps of loop, and you can safelly pass them to another anonymous function (this at setTimeout)

Your original version didn't worked because:

  • your function passed to setTimeout holds reference to variable i (not value of it), and it gets value when this function is called, which is not at time of adding it to setTimeout
  • the value of this variable gets changed in loop, and before you even get first setTimeout it gets endOpacity value (last value from for loop)

Unfortunately JavaScript only has function scope, so it won't work if you create variable inside the loop and assign new actual value, because whenever there is some var inside function, those variables are created at time of function execution (and get undefined value by default). The only (easy) way to create new scope is to create function (may be anonymous) and create new variables inside them (parameters are variables too)

MBO
+1 - I've also started with an anonymous function. looking again, I guess yours is more elegant.
Kobi
can you explain the thing with the (i) ?
ronik
@ronik I updated my answer
MBO