Javascript code is wrong i don't know why

Question

why can't this code work ? I'm trying to change an image every 4 seconds... I really don't understand, everything should work correctly.

<!DOCTYPE html>
<html>
<head>
<script type='text/javascript'>
function Falcon0408()
{
document.getElementByTagName('img');


if (document.getElementByTagName('img').src='img1.png')
{
document.getElementByTagName('img').src='img2.png';
}




else if (document.getElementByTagName('img').src='img2.png')
{
document.getElementByTagName('img').src='img3.png';

}

else if (document.getElementByTagName('img').src='img3')
{
document.getElementByTagName('img').src='img1.png';
}
}
</script>

</head>
<body>
<script>
setTimeout(Falcon0408(),400);
</script>
<img src='img.png'/>


</body>
</html>

`setTimeout(Falcon0408,400);` <-- don't invoke the function, `setTimeout` expects a function reference — Andrew Whitaker, Sep 24 '14 at 23:10
for starters, `getElementByTagName` does not exist, and `getElementsByTagName` returns an array. — njzk2, Sep 24 '14 at 23:11
Also src is 'img.png' which isn't 'img1.png', 'img2.png' or 'img3'. — Robert, Sep 24 '14 at 23:18
Anyways, invoking setTimeout only once will only invoke the function once. — Robert, Sep 24 '14 at 23:21

score 2 · Answer 1 · edited May 23 '17 at 12:14

It's getElementsByTagName - "elements" is plural because you get all of the ones with that tag.
You're using the assignment operator = instead of comparison == or ===.
The actual image source in the HTML is just "img.png" so it will never match any of your "if" statements.
Not an error, but a tip: "Don't Repeat Yourself." Get the image's attribute once, and store it in a variable, then check against that variable.
Give your image tag a unique ID so you can target it directly with getElementById (note it's "Id" not "ID"). What if you have multiple img tags on the page? You probably don't want all of them to change.

Code:

<img id="myImg" src="img1.png"/>

function Falcon0408()
{
    var image = document.getElementById('myImg');

    if (image.src === 'img1.png')
   {
       image.src = 'img2.png';
   }

   else if (image.src === 'img2.png')
   {
       image.src='img3.png';
   }

   else if (image.src === 'img3')
   {
      image.src='img1.png';
   }
}

What will you do when you want to cycle through 20 or 1000 images? Will you keep adding if/else blocks?

A better approach would be to have an array of images (or just know exactly how many photos you have in the folder) and use a loop to change the source based on the number of images in the array. That way you don't have to keep adding else/if statements every time you want to add a new image ... but that's beyond the scope of this question.

If you want this to loop, this won't work. See this answer for how to use setInterval(), or how to include another call to setTimeout() inside the function so that it keeps going.

Added comment re: timeout issue - thanks for catching the other error too btw :) — mc01, Sep 24 '14 at 23:59

Javascript code is wrong i don't know why

1 Answers1