CODEWITHSUNDEEP
javascript
unixia
unixia

Reputation: 4350

Displaying images JavaScript

I've tried running this code a lot of times. Images are not even getting loaded. Can I get a good reason for that? html file and images are saved in the same directory and there are two images.

<script language="JavaScript">
delay = 200;
image_seq = new Array();
for (i = 1; i < 3; i++) {
    image_seq[i] = new image();
    image_seq[i].src = i + ".jpeg"
}
num = 1;

function animate() {
    document.pic_rotate.src = image_seq[num].src;
    num++;
    if (num > 2)
        num = 1;
}

function slow() {
    delay = delay + 25;
    if (delay > 2000)
        delay = 2000;
}

function fast() {
    delay = delay - 25;
    if (delay < 0)
        delay = 0;
}
</script>
<img name="pic_rotate" src="1.jpeg" onLoad="setTimeout('animate()',delay)">
<form>
<input type="button" value="Slow" onClick="slow()">
<input type="button" value="Fast" onClick="fast()">
</form>

Upvotes: 0

Views: 849

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1075785

There are lots of issues with that code, but the main reason you're not seeing images is that this line uses image where it should use Image:

image_seq[i] = new Image();
//                 ^---- Note that this is `I`, not `i` -- capitalization matters

Other issues with the code include:

  1. You're falling prey to The Horror of Implicit Globals all over the place, by failing to declare your variables.

  2. You're relying on the browser dumping the identifier pic_rotate onto document because you've given the element a name attribute. That's not necessarily reliable. Instead, use an id and look up the element explicitly using document.getElementById.

  3. Your first call to animate will just assign the same image to pic_rotate that it already has, so you'll seem to have an unusually long delay before the first image arrives.

  4. The code using a counter and an array seems unnecessarily complicated just to flip between two images (1.jpeg and 2.jpeg).

  5. new Array(), while not in any way wrong, is better written simply as [].

  6. There's no reason to create image elements if you're only going to use their src property (what you're doing won't, for instance, prefetch them because you never add the images you're creating to the DOM).

  7. Using old DOM0 onXyz attributes is again not wrong, but generally not best practice.

  8. Passing strings into setTimeout is poor practice.

  9. It's harmless, but there's no reason to put the buttons in a form if you're not submitting the form.

Here's a complete example dealing with the above: Live Copy I've stuck with the loop on the theory you probably had more than two images you were planning to use...

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <title>Testing</title>
</head>
<body>
<div id="imageContainer">
</div>
<input id="buttonSlow" type="button" value="Slow">
<input id="buttonFast" type="button" value="Fast">
<script>
  // A scoping function to avoid using any globals
  (function() {
    "use strict"; // Improve JavaScript using its new "strict" mode

    // Declare our variables
    var images, index, delay;

    // Our main initialization function
    function init() {
      // Get the image container
      var imageContainer = document.getElementById("imageContainer");

      // Create an array of images
      images = [
        createImage("https://www.gravatar.com/avatar/1d5ac1c9d660351786dc6b3873627c4d?s=128&d=identicon&r=PG&f=1"),
        createImage("https://www.gravatar.com/avatar/ca3e484c121268e4c8302616b2395eb9?s=128&d=identicon&r=PG")
      ];

      // Add our images to the container, with only the first one being visible
      for (index = 0; index < images.length; ++index) {
        if (index > 0) {
          images[index].style.display = "none"; // Even though hidden, will get fetched
        }
        imageContainer.appendChild(images[index]);
      }

      // Set our index to the currently-showing image, and our standard delay
      index = 0;
      delay = 500; // 500ms = half a second

      // Hook up our buttons (look into addEventListener [attachEvent on
      // older IE], but onclick works for this example)
      document.getElementById("buttonSlow").onclick = function() {
        delay = Math.min(delay + 25, 2000); // Don't go past 2000
      };
      document.getElementById("buttonFast").onclick = function() {
        delay = Math.max(delay - 25, 0); // Don't go below 0
      };

      // Start our animation
      setTimeout(animate, delay);
    }

    // A function to create images
    function createImage(src) {
      var img = document.createElement('img');
      img.src = src;
      return img;
    }

    // Our animation function
    function animate() {
      images[index].style.display = "none"; // Hides it
      index = (index + 1) % images.length;  // A fancy way to add one and wrap back to 0 if needed
      images[index].style.display = "";     // Shows it

      // Hook up the next animation with our current delay
      setTimeout(animate, delay);
    }

    // Start!
    init();

  })();
</script>
</body>
</html>

Upvotes: 4

Related Questions