HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

JavaScript Image Preloading Script

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptimagescriptpreloading

Problem

I wrote an infinite scrolling scrolling plugin for an app I'm developing. When requesting the second 'page' from the server, I loop through each image, and give it a very basic onload function.

// I will have the result in a variable called data
// Fetch each image from result
$images = $( data.content ).find('img');
// Cache length for use in for loop
imagesLength = $images.length;

for ( var i = 0; i < imagesLength; i++ ) {
    // Create new Image object
    img = new Image();
    // Match src of image in the result
    img.src = ( $images[i].src );
    // Add onload listener
    img.onload = function () {
        // Increment iterator, and check if we've loaded all the images
        if( ++imageLoadingIterator == images.length) 
            _continue(); // Run _continue() if this is the case
    }
}


I feel like this isn't the best way to do this. Is there a way I can improve this function to run faster? For example: I'm thinking maybe creating a new Image object for each img tag is too much overhead.

Solution

Not really about performance, but this is more about clarity of the code.

-
Attach onload handlers first, for readability. Appending the src starts the loading process. If you saw code that loaded something, expect to do something, but haven't found the handler to do it makes it a bit confusing.

-
Declare one handler function in a persistent scope and have the onload refer to it, rather than create one handler for each onload, or creating the function everytime the whole routine executes.

-
Instead of find(), use jQuery function's second parameter, context. With this, jQuery finds the elements with the given selector under the jQuery object that is on the second parameter.

-
If the images are children of data.content, use children() instead of find() to avoid deep DOM traversal. Also, the lower you are in the DOM, the less deep find() has to dive and find your elements.

-
Use strict comparison as much as possible.

-
Don't forget to declare the variables and use var. You'd be declaring globals if you didn't.

And so:

var $images = $('img', data.content),
  imagesLength = $images.length,
  imageLoadingIterator = 0,
  img, i;

function onImageLoad() {
  if (++imageLoadingIterator === images.length) _continue();
}

for (i = 0; i < imagesLength; i++) {
  img = new Image();
  img.onload = onImageLoad;
  img.src = $images[i].src;
}


However, these advices might have a very negligible gain in performance, and might be unnecessary since the compiler might be optimizing them in some way.

Code Snippets

var $images = $('img', data.content),
  imagesLength = $images.length,
  imageLoadingIterator = 0,
  img, i;

function onImageLoad() {
  if (++imageLoadingIterator === images.length) _continue();
}

for (i = 0; i < imagesLength; i++) {
  img = new Image();
  img.onload = onImageLoad;
  img.src = $images[i].src;
}

Context

StackExchange Code Review Q#23587, answer score: 4

Revisions (0)

No revisions yet.