Uncaught TypeError cannot set property 'src' of null

1
votes

So on my new website in google chrome, I am trying to make the images switch every 5 seconds using a setInterval function. This seems to work, however I have the problem cannot set property src of null.

var images = new Array();
images[0] = "/images/grilled-chicken-large.png";
images[1] = "/images/sizzly-steak.png";
var img = document.getElementById("img");

function changeImage() {
     for (var i = 0; i < 3; i++) {
        img.src = images[i];
        if (i == 2){i = i - 2;};
     }
}

window.onload=setInterval("changeImage()", 5000);

I know the problem is that I am trying to get the element with an id of img before the page is done loading, but I've already got a window.onload, so i don't know what to do. Also, if i make an init() function containing the setInterval and the img variable, the page freezes.

Any ideas?

4
javascripthtml
You only have images[0] and images[1], but you're trying to access images[2]. – bfavaretto
Side note: don't pass strings to setInterval, it uses eval. Pass functions: setInterval(changeImage, 5000);. Also, window.onload = setInterval(changeImage, 5000); doesn't do what you think it does. – Rocket Hazmat
yea just realized that bfavaretto, thanks. Rocket, do you mean just pass it as changeImage() instead of "changeImage()"? And what do you mean by the last line not doing what I think it does? – Ilan Biala
@Ilan: Bit too much for comments, please create separate questions for them. The last line executes setInterval, immediately then assigns the result of that method (it's an integer used as ID) to window.onload, which does not make sense. See my anwer, I've added the onload stuff. – Yogu

4 Answers

3
votes

The problem is that you access element 2 of an array with only two elements (0 and 1).

But there is also a logical error in your script: Where do you store the index of the currently visible image? You need a global variable which holds that index.

var currentIndex = 0;
function changeImage() {
    // Switch currentIndex in a loop
    currentIndex++;
    if (currentIndex >= images.length)
        currentIndex = 0;

    var img = document.getElementById("img");
    img.src = images[currentIndex];
}

document.onload = function() {
    setInterval(changeImage, 5000);
}

I've moved the img variable into the function so that it is assigned after five seconds when the document has loaded.

Just to clear things up: JavaScript is an event-based programming language. That means that the slideshow-change code is executed every time the interval fires, does some work (switch to the next slide) and then is done, so that the browser can render the page. If you iterate through the slides in a loop, there is no way for the browser to show the new slide because the script is still executing between the slides. The solution is what I've posted: Define a global variable that replaces the loop variable, and increment it every time the next slide is to be shown.

1
votes

Here:

window.onload = function () {

    var images = [
        'http://placekitten.com/100/200',
        'http://placekitten.com/200/100',
    ];

    var img = document.getElementById( 'img' );

    setInterval(function () {
        img.src = img.src === images[0] ? images[1] : images[0];
    }, 1000 );

};

Live demo: http://jsfiddle.net/wkqpr/

If you have more than two images, you can do this:

window.onload = function () {

    var images = [
        'http://placekitten.com/100/200',
        'http://placekitten.com/200/100',
        'http://placekitten.com/200/150',
        'http://placekitten.com/150/300'
    ];

    images.current = 0;

    var img = document.getElementById( 'img' );

    setInterval(function () {
        img.src = images[ images.current++ ];
        if ( images.current === images.length ) images.current = 0;       
    }, 1000 );

};

Live demo: http://jsfiddle.net/wkqpr/1/

0
votes

Your for loop looks odd. Try that loop:

function changeImage(){
  if(!img.attributes["index"]){img.attributes["index"]=0;return;};
  img.attributes["index"]++;
  img.src=images[img.attributes["index"]];
}
window.onload=function(){
  window.img=document.getElementById("img");
  setInterval("changeImage()",5000)
}

Should work.
You'll notice that I store the image index in an attribute of the image, as opposed to a global variable. I think the global is slightly faster, but storing it in the image means you can easily expand this to multiple images if needed. Also, the problem with your browser freezing was that the following code:

for(var i=0;i<3;i++){
  //Some more stuff
  if(i==2){i-=2};
}

This loop is infinite, because whenever i reaches 2, i==2 becomes true, which means that iteration will reset i to 0 and start it all over again. The only way to prevent that would be a break; somewhere, which I don't see anywhere. Tip: It is generally a bad idea to tamper with the index variable in for loops.

0
votes
function changeImage() 
{
    for (var i = 0; i < 2; i++) 
    {
        img.src = images[i];
     }
}


window.onload=function()
{
    setInterval(changeImage,5000);
}

Your last line in the for loop is completely useless and just complicate things, Also, Your way of setting a window onload event is not standard!

EDIT: the src property is null because , obviously, your array size is only 2!!