2

I can't figure out what is wrong with this code. Firefox's error console tells me: "this.animateImgSlider is not a function".

What I would like is to call this.selectImage() with jsItems[0].selectImage(0) and then have this.animateImgSlider() be called some number of times until selfCall is false:

function WindowItem(inId) { this.id = inId; this.imgSliderTarget = 0; this.imgSlider = document.getElementById("imgSlider"+inId); this.animateImgSlider = function() { var selfCall = true; var currLeft = parseInt(this.imgSlider.style.left, 10); if (currLeft < this.imgSliderTarget) { currLeft += 8; if (currLeft >= this.imgSliderTarget) { currLeft = this.imgSliderTarget; selfCall = false; } } else { currLeft -= 8; if (currLeft <= this.imgSliderTarget) { currLeft = this.imgSliderTarget; selfCall = false; } } this.imgSlider.style.left = currLeft+"px"; if (selfCall) setTimeout("this.animateImgSlider()", 0); } this.selectImage = function(inImg) { this.imgSliderTarget = -inImg*488; this.animateImgSlider(); } } var jsItems = new Array(); jsItems.push(new WindowItem(0));

This line is the one that throws the error:

if (selfCall) setTimeout("this.animateImgSlider()", 0);
Improve this question

2 Answers 2

Reset to default
5

Use this instead:

if (selfCall) { var self = this; setTimeout(function () { self.animateImgSlider(); }, 0); }

(of course, it would make more sense to me to declare self at the top and use that throughout the function instead of this)

A string that is passed to setTimeout to execute is done so in the global scope, meaning this refers to window. It is not executed in the current scope, where the setTimeout call is.

On a side note, as a suggestion, I would move your animateImgSlider and selectImage methods outside, and into the WindowItem prototype, so that they can be shared by all instances, instead of creating a new anonymous function each time. So for example, you'd have:

function WindowItem(inId) { // blah blah } WindowItem.prototype.animateImgSlider = function () { // blah blah }; WindowItem.prototype.selectImage = function(inImg) { // blah blah };

The value of this will still refer to the specific instance, unless of course you attempt to use it in a string for the setTimeout parameter :)

This is clearly not required, as your code currently works fine, but it is a commonly used convention for declaring methods on a constructor function for instances to share.

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

4 Comments

@asimes No problem, glad it worked! I just updated my answer with some extra random things you might want to look at.
Not quite sure I understand the advantage of using prototype, what is being created each time in my code?
Prototype is a must if your object will be created many times, instead of recreating the object everytime in full, your object will inherit its prototype that are shared across all instances.
@asimes The methods created with prototype are done once and reference one place. With your code, every time you call new WindowItem(), the function is called, meaning it creates a new object and sets properties (and methods) by using this.X = Y for that specific new object. So every time that happens, it creates a new anonymous function and assigns it to animateImgSlider and another to selectImage. With prototype, it creates a property/method inherited by all objects constructed with new WindowItem(), and references a single anonymous function.
2

You can use bind .here is the link 

if (selfCall) { setTimeout(function () { this.animateImgSlider(); }.bind(this), 0); }
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.