0

I am beginning to hate objects in javascript.

Every time I have error and I fix it, a new error appears, and so on.
Can you please take a look at the following code and tell me what's wrong ?

problem message:
"this.Images is undefined" and more errors also

HTML File

<div id="SlideShow" >
   <img id="img" src="images/img.jpg" alt="" /><span id="desc"></span>
</div>

<script type="text/javascript">
    meToo.Images = ['images/img.jpg','images/img2.jpg','images/img3.jpg','images/img4.jpg','images/img5.jpg'];
    meToo.Titles = ['Pic1','pic2','Pic3','Pic4','Pic5'];
    meToo.Play('img');
</script>  

Javascript Object

var meToo = {
     Images: [],
     Titles: [],
     counter: 0,
     Play: function(ElemID){
         var element = document.getElementById(ElemID);
         var ImgLen = this.Images.length;

         if(this.counter < ImgLen){
             this.counter++;
             element.src = this.Images[this.counter];
             element.nextSibling.innerHTML = this.Titles[this.counter];
         }else{
             this.counter = 0;
         }
         setTimeout(this.Play, 1000);
    }   
};

See the Example

Lion King
  • 32,851
  • 25
  • 81
  • 143

3 Answers3

3

See this question. Otherwise setTimeout sets this to the window object. Also, the counter should be incremented after setting the images or you will be reading outside the array bounds.

Finally, when resetting the counter to 0, there will be an additional one-second delay before the loop restarts, because the image is not being reset in that else block. You may wish to rewrite that part of the logic.

Updated Fiddle

    if(this.counter < ImgLen){
        element.src = this.Images[this.counter];
        element.nextSibling.innerHTML = this.Titles[this.counter];
        this.counter++;
    }else{
        this.counter = 0;
    }
    var _this = this;
    setTimeout(function() { _this.Play('img') }, 1000);

This is what I would write to keep the loop going at one-second intervals:

Play: function(ElemID) {
    var element = document.getElementById(ElemID);
    var ImgLen = this.Images.length;

    if (this.counter == ImgLen) {
        this.counter = 0;
    }
    element.src = this.Images[this.counter];
    element.nextSibling.innerHTML = this.Titles[this.counter];
    this.counter++;

    var _this = this;
    setTimeout(function() {
        _this.Play('img')
    }, 1000);
}
Community
  • 1
  • 1
Tina CG Hoehr
  • 6,721
  • 6
  • 44
  • 57
  • I want to know, why we do that: "var _this = this"; and "_this.Play('img')", why use "this" direct not work ? – Lion King Jun 23 '12 at 20:25
  • You want to refer to the `Play` function of meToo, but setTimeout will have been called by the global, `window` object that doesn't have a `Play` function. By setting a reference to meToo with `_this`, the `_this.Play` function (referencing meToo) will be called after 1000 milliseconds. In this case writing `meToo.Play()` inside setTimeout [will work too](http://jsfiddle.net/f56gk/15/), in the question I linked the scenario needed a variable like `_this` is needed and works as a more general case. – Tina CG Hoehr Jun 23 '12 at 20:37
  • Thanks **Mr MaryAnne** for help, I have another simple question in same subject, as you know and I'm sorry to bother you, I have identified in `setTimeOut` function every 1 second will change the picture, but why when changing from last picture to first picture take more time< I think more than 1 second? – Lion King Jun 23 '12 at 20:55
  • Because the way the function is written, it only follows the `if` or the `else` once every second. So when the array is finished, it sets the counter back to zero and does nothing else (the image stays still). Then the next time the function is visited the image changes. I proposed a change on the last part of my answer to correct this, the other answers mention this also. – Tina CG Hoehr Jun 23 '12 at 21:00
  • thank you very much **Mr MaryAnne** for help, Although I don't understand the last point, but you are Respectable man – Lion King Jun 23 '12 at 21:07
  • Which point was not clear? About the counter resetting? Or the alternative I suggested? And please, no need to call me Mr., MaryAnne will do :) – Tina CG Hoehr Jun 23 '12 at 21:10
  • Ok, the point about, more time when change from last pic to first pic. – Lion King Jun 23 '12 at 21:14
  • The function `Play` will choose between the `if` block and the `else` block depending on the value of `counter`. Notice that only in the `if` block will the picture change by setting the `element.src`. When the counter exceeds the array limit, it will reset to zero, but the picture won't change. So it sits on the last image until the next time the function runs. Remember that the setTimeout doesn't care what the counter is, it will keep the delay. Either add the `element.src` bit to the `else` block or rewrite the logic, separating the counter reset like I did. – Tina CG Hoehr Jun 23 '12 at 21:34
2
if(this.counter < ImgLen)

is wrong.

What will happen here is that when you run

this.counter++;

the value of that variable will now be ImgLen.length

Arrays in javascript go from 0 to length -1.So now you'll be exceeding the array's length, when you run:

this.Images[this.counter];

and encounter an error.

The quick fix here is to change to

if(this.counter < ImgLen -1) 

If you're encountering other problems, then post the exact error message. (Run in Chrome and press F12 (for example) to bring up the console so you can see the errors).

Nik
  • 2,718
  • 23
  • 34
  • Thanks, but this is not the main problem – Lion King Jun 23 '12 at 17:52
  • Then you'll need to explain exactly what your problem is, and the errors you are seeing. – Nik Jun 23 '12 at 17:54
  • I can see now that your "this" property refers to Window the second time it executes, so it no longer refers to meToo, so Images will now be null... – Nik Jun 23 '12 at 17:57
0

Here check this out.It should work perfectly for you.I have made it a singleton object and also I am doing a check if incase meToo.Play is called before the dom is loaded it will not crash.All the other mistakes that the guys above are pointing are also taken care of.

    <script>
    var meToo = function(){
    var Images = ['http://www.image-upload.net/di/WMPI/img.jpg','http://www.image-upload.net/di/HPUQ/img2.jpg','http://www.image-upload.net/di/WQ9J/img3.jpg','http://www.image-upload.net/di/GIM6/img4.jpg','http://www.image-upload.net/di/0738/img5.jpg'];
    var Titles = ['Pic1','pic2','Pic3','Pic4','Pic5'];
    var counter = 0;
    return {
        Play: function(ElemID) {
            var element = document.getElementById(ElemID);
            if(element){
                var ImgLen = Images.length;

                if(counter < ImgLen) {
                    element.src = Images[counter];
                    element.nextSibling.innerHTML = Titles[this.counter];
                    counter++;
                } else {
                    counter = 0;
                }
            }
            setTimeout(callmeToo, 1000);
        }
    }
    }();

    function callmeToo(){
        meToo.Play('img');  
    }

    callmeToo();
</script>
Harshniket Seta
  • 654
  • 7
  • 7