PDA

View Full Version : A Big Problem



???
07-01-2007, 02:45 PM
I am having problems with my script. It's meant to animate an image by cycling through every certain duration. But it keeps saying at line 20 I have a problem. Supposedly my problem is that animationStages[5] is not a function, but it shouldn't be anyway! My code so far is:
<html>
<head>
<title>

</title>
<script type="text/javascript">
var animationStages = new Array ();
var animationEnvironment = function () {
var data = new Array (new Array (),new Array ());
for (i = 0;i < document.getElementById ("data").innerHTML.split ("{").length; i ++) {
data[0][i - 1] = document.getElementById ("data").innerHTML.split ("{")[i].split ("}")[0].split (":")[0];
data[1][i - 1] = document.getElementById ("data").innerHTML.split ("{")[i].split ("}")[0].split (":")[1];
}

for (i = 0;i < data[0].length;i ++) {
if (i != data[0].length - 1) {
animationStages[i] = function () {
document.getElementById ("daImage").src = data[0][i];
setTimeout ("animationStages[" + (i + 1) + "] ();",data[1][i]);
}
}
if (i == data[0].length - 1) {
animationStages[i] = function () {
document.getElementById ("daImage").src = data[0][i];
setTimeout ("animationStages[0] ();",data[1][i]);
}
}
}
animationStages[0] ();
}
window.onload = function () {
animationEnvironment ();
}
</script>
</head>
<body>
<div id="data">
{hi.png:1000}
{bi.png:1000}
{lo.png:1000}
{bi.png:1000}
</div>
<img id="daImage">
</body>
</html>

I have the images in the same directory. Can you help me?

mwinter
07-01-2007, 03:59 PM
Supposedly my problem is that animationStages[5] is not a function, but it shouldn't be anyway!

True, but the question that you should be pondering is: "Why is my code trying to access index 5 in the first place?" The answer is simple in principle, but not necessarily simple to understand if you aren't familiar with closures.



My code so far is:

In future, please use the
... or
... vbCode tags when posting code.

There are a few other things to note regarding your code that I'll discuss as I go.



var animationEnvironment = function () {

On the whole, use function declarations when creating functions.



function animationEnvironment() {

Reserve function expressions for occasions where you need a function object to be created conditionally, anonymously, or when assigning a function object directly to a variable or property.





var data = new Array (new Array (),new Array ());
for (i = 0;i < document.getElementById ("data").innerHTML.split ("{").length; i ++) {
data[0][i - 1] = document.getElementById ("data").innerHTML.split ("{")[i].split ("}")[0].split (":")[0];
data[1][i - 1] = document.getElementById ("data").innerHTML.split ("{")[i].split ("}")[0].split (":")[1];
}


In that loop, you call the split method seven times, and getElementById three times. Save intermediate results! You also create a property, -1, on the first iteration.

If you perform parsing, I would suggest that you use regular expressions: it's simpler:


var elements = document.getElementById('data').innerHTML.match(/{[^:}]+:[^}]+}/g),
data = [];
for (var i = 0, n = elements.length; i < n; ++i) {
var result = /{([\w.-]+):(\d+)}/.exec(elements[i]);
data.push([result[1], result[2]]);
}

though the code above contains no error handling. What I actually suggest, though, is that you place the data in the script itself and avoid parsing altogether.

Now we come to the real problem:




for (i = 0;i < data[0].length;i ++) {
if (i != data[0].length - 1) {
animationStages[i] = function () {
document.getElementById ("daImage").src = data[0][i];
setTimeout ("animationStages[" + (i + 1) + "] ();",data[1][i]);
}
}
if (i == data[0].length - 1) {
animationStages[i] = function () {
document.getElementById ("daImage").src = data[0][i];
setTimeout ("animationStages[0] ();",data[1][i]);
}
}
}


A closure is formed when an inner function survives after the outer function returns. In this case, animationEnvironment is the outer function, and the two function expressions above are the inner functions. Because you assign a reference to an array, the function object is not garbage-collected when animationEnvironment returns. This prevents all of the variables that can be read by the inner function from being garbage-collected, too, allowing them to be read long after animationEnvironment has returned.

A frequent mistake, however, is not realising (or forgetting) that local variables are properties of objects (or at least act that way), and these objects (called variable objects) will be shared by any function that's in scope. This means that changes to the local variables can be observed by all sharing functions.

In your case, the change is made by the loop: on each iteration, it increments the (global!) variable, i. At the end of the loop, i has been incremented to 5 and all of the function objects created read that, too.

Read that last paragraph again: the first function doesn't read i to be 0, but 5!

One solution is to isolate each function object by creating each one through a function:



function animationEnvironment() {
var image = document.images.daImage;
/* Initialise data... */

for (var i = 0, n = data.length; i < n; ++i)
animationStages[i] = createStageTask(i);
animationStages[0]();

/* Clear up any DOM object references! */
this.onunload = function () {
image = null;
data.length = 0;
};

function createStageTask(index) {
return function () {
image.src = data[index][0];
setTimeout('animationStages[' + ((index + 1) % data.length) + ']()', data[index][1]);
};
}
}