JavaScript Code for image rotation on a web page

How does this look for basic image rotation? What is there to be improved here?

    
  
  
  
  
  
    

  
      var rotation = function () {
          var currentImage;
          var count = function () {              
              // Figure out how many images we have on the rotation          
              var x = 0;
              while (document.getElementById('rotateImg' + (x + 1).toString()))
                  x++;
              return x;
          } ();

          var hideImages = function () {
              // Hide all the images              
              for (var i = 0; i 

Three things I noticed:

1. Looking through the document with DOM methods is a slow process. There's often no way around it, and it won't crash anything here, but try to minimize it when you can. From page 183 of Stoyan Stefanov's JavaScript Patterns:

DOM access is expensive; it's the most common bottleneck when it comes
to JavaScript performance. This is because the DOM is usually
implemented separately from the JavaScript engine...


The bottom line is that DOM access should be reduced to a minimum.
This means:

• Avoiding DOM access in loops

• Assigning DOM references to local variables and working with the locals

• Using selectors API where available

• Caching the length when iterating over HTML collections

I've done that here. I created one extra variable named images and assigned it an empty array. Then, using your counting function, I store the image references in it.

var currentImage,
    images = [],

      ...

    count = (function () {              
      // Figure out how many images we have on the rotation          
      var x = 0;
      while (document.getElementById('rotateImg' + (x + 1).toString())) {
        images[x] = document.getElementById('rotateImg' + x.toString());
        x++;
      }
      return images.length;
    }()),

(If you're able to change the HTML, you can avoid the loop altogether and collect these images in one line. Just add an id to the element that contains them...

  
  ...

...and collect them with...

var images = document.getElementById("container").getElementsByTagName("img");

But this doesn't help you if you can't change the markup.)

Now you can run hideImage and showImages on the array instead of having to walk the DOM every time.

1. When the first argument to setInterval is a string, the browser runs eval() -- an implementation of the JavaScript compiler -- on it before firing. That's also expensive and unnecessary here. From page 111 of Douglas Crockford's JavaScript: The Good Parts:

The browser provides setTimeout and setInterval functions that can
take string arguments or function arguments. When given string
arguments, setTimeout and setInterval act as eval. The string
argument form also should be avoided.

Getting rid of the quotes and the closing parentheses makes it a plain old function reference, which works fine.

So I changed this:

var interval = setInterval("rotation.advanceRotation()", 4000);

To this:

var interval = setInterval(rotation.advanceRotation, 4000);

1. Not crucial here. You can do all your variable declaration with one var statement per function. This reduces the chances of your variables being overwritten or unavailable due to hoisting. From JavaScript Patterns again:

JavaScript enables you to have multiple var statements anywhere in a
function, and they all act as if the variables were declared at the
top of the function... This can lead to logical errors when you use a
variable and then declare it further in the function.

So I took out the multiple var statements and declared them all at the top of the outer function.

The final code:

  
  Rice Flour Cookies' image rotation test

  

  
        
      
      
      
      
      
        
    
  

  var rotation = function () {
    var currentImage,
        images = [],
        count,
        hideImages,
        showImage,
        fn;

        count = (function () {              
          // Figure out how many images we have on the rotation          
          var x = 0;
          while (document.getElementById('rotateImg' + (x + 1).toString())) {
            images[x] = document.getElementById('rotateImg' + x.toString());
            x++;
          }
          return images.length;
        })();

        hideImages = function () {
          // Hide all the images              
          for (var i = 0; i 

And you can see it at work here:
http://james.da.ydrea.ms/riceflourcookies/rotate.html

StackExchange Code Review Q#5784, answer score: 3