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

Preloading images in JavaScript

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

Problem

I'm working on a little game where you press a button and a new image appears. There's a glitch every time you ask a new image to display and it occurred to me that I should preload the necessary images. However, I'm not sure how to do this efficiently as I'm not using a variable to display the images. It's also been pointed out to me that I should look into storing reference for the elements, but I don't know how to go about doing so. This is what I'm working on. Any suggestions?

HTML:


        
        
    


JavaScript:

```
// Calls the loadJajo function and passes the image URL
// Initiates directionSlide function
function setup() {
loadJajo('jajo.png');
elem = document.getElementById('directions');
directionSlide();
}

//Creates a new image object (Jajo) and writes it to the page.
function loadJajo(jajoSRC) {
var main = document.getElementById('jajo'); // Creates an variable to represent the "main" division
var defaultJajo = document.createElement('img'); // Creates a new image object (default Jajo image)
defaultJajo.src = jajoSRC; // adds the source file name to the defaultJajo image object
main.appendChild(defaultJajo); //puts the defaultJajo object inside the "main" division
}

// Listen for key pressed events.
document.onkeydown = function(event) {
var keyPress = String.fromCharCode(event.keyCode); // Assigns value of key pressed to variable.

if(keyPress == "W") { // If 'W' is pressed, display Jajo waving.
document.getElementById("jajo").innerHTML= "";
document.onkeyup = function(event) { // If 'W' is released, display default Jajo.
document.getElementById("jajo").innerHTML= "";
}
} else if(keyPress == "A") { // If 'A' is pressed, display Jajo winking.
document.getElementById("jajo").innerHTML= "";
document.onkeyup = function(event) { // If 'A' is released, display default Jajo.
document.getElementById("jajo").innerHTML= "";
}
} else if(keyPress == "S") { // If 'S' is pressed, display transparent Jajo.

Solution

From a once over:

-
Please indent your functions, bad:

function setup() {
loadJajo('jajo.png');
elem = document.getElementById('directions');
directionSlide();
}


better:

function setup() {
    loadJajo('jajo.png');
    elem = document.getElementById('directions');
    directionSlide();
}


  • Consider having only 1 comma separated var statement on the top of your code



-
Lighten up on the comments, you are overdoing it, this:

//Creates a new image object (Jajo) and writes it to the page.
function loadJajo(jajoSRC) {
var main = document.getElementById('jajo'); // Creates an variable to represent the "main" division
var defaultJajo = document.createElement('img'); // Creates a new image object (default Jajo image)
defaultJajo.src = jajoSRC; // adds the source file name to the defaultJajo image object
main.appendChild(defaultJajo); //puts the defaultJajo object inside the "main" division
}


could be this:

//Creates a new image object (Jajo) and writes it to the page.
function loadJajo(jajoSRC) {
  var main = document.getElementById('jajo'), 
      defaultJajo = document.createElement('img'); 
  defaultJajo.src = jajoSRC; 
  main.appendChild(defaultJajo); 
}


-
You are actually storing a reference to an element here:

elem = document.getElementById('directions');


However, you should call elem differently, probably directions. Also, you should consider caching document.getElementById('jajo') since you use it a ton of times.

-
There is a clear mapping in your code between key presses and images, you could write your keydown handler like this:

var keyImageMap = 
{
  'W' : 'jajo_wave.png',      // If 'W' is released, display default Jajo.
  'A' : 'jajo_wink.png',      // If 'A' is pressed, display Jajo winking.
  'S' : 'jajo_invisible.png', // If 'S' is pressed, display transparent Jajo.
  'D' : 'jajo_purple.png',    // If 'D' is pressed, display purple Jajo.
  'E' : 'jajo_carrot.png'     // If 'E' is pressed, display Jajo eating a carrot.
}

// Listen for key pressed events.
document.onkeydown = function(event) {
  // Assigns value of key pressed to variable.
  var keyPress = String.fromCharCode(event.keyCode); 

  if( keyImageMap[ keyPress ] )
  {
    document.getElementById('jajo').innerHTML = '';
  }
}


  • In the previous sample code you should of course have used a reference to jajo instead of calling document.getElementById. Also you should have deep thoughts on rewriting that image tag every time, perhaps it would be better to have a reference to the image and then keep changing src

Code Snippets

function setup() {
loadJajo('jajo.png');
elem = document.getElementById('directions');
directionSlide();
}
function setup() {
    loadJajo('jajo.png');
    elem = document.getElementById('directions');
    directionSlide();
}
//Creates a new image object (Jajo) and writes it to the page.
function loadJajo(jajoSRC) {
var main = document.getElementById('jajo'); // Creates an variable to represent the "main" division
var defaultJajo = document.createElement('img'); // Creates a new image object (default Jajo image)
defaultJajo.src = jajoSRC; // adds the source file name to the defaultJajo image object
main.appendChild(defaultJajo); //puts the defaultJajo object inside the "main" division
}
//Creates a new image object (Jajo) and writes it to the page.
function loadJajo(jajoSRC) {
  var main = document.getElementById('jajo'), 
      defaultJajo = document.createElement('img'); 
  defaultJajo.src = jajoSRC; 
  main.appendChild(defaultJajo); 
}
elem = document.getElementById('directions');

Context

StackExchange Code Review Q#45162, answer score: 3

Revisions (0)

No revisions yet.