[RESOLVED] Tidy up javascript

This site is best viewed in a modern browser with JavaScript enabled.

13thwarrior

Better way to rewrite this.

I’m reusing a script. I've added the small helper function up front and I’ve rewritten the first function(swapImage). The second function(addLoadEvent) seems a little verbose. As a whole the code works, but it seems the second function could be done more succinctly. Can anybody help? Here’s the code.

var $ = function (id)
	{ return document.getElementById(id);
	}

var	grpImages = ["images/blu.png",
				 "images/org.png",
				 "images/prp.png",
				 "images/ylw.png",
				 "images/grn.png",
				 "images/red.png"];

var grpCaptions = ["Blue Suede",
				   "Orange Peel",
				   "Purple Rain",
				   "Lemon Yellow",
				   "Kelly Green",
				   "Scarlet Red"];

var i = 0; 
var j = grpImages.length-1;    

var swapImage = function()
{ 
  var imgCase = $("slide");
  //imgCase.src = image[i];  // need this line for IE 8
  imgCase.setAttribute( "src", grpImages[i] ); // comment out this line for IE8

  var captionBox = $("myDiv"); 
  captionBox.innerHTML = grpCaptions[i];

if( i < j )
	{ i += 1;
	}

else { i = 0;
	 } 
setTimeout(swapImage, 2500);  
}

function addLoadEvent(func)
{ var oldonload = window.onload;

if (typeof window.onload !== 'function')
	{ window.onload = func;
	}

else { window.onload = function()
		{ if (oldonload)
			{ oldonload();
			}
				func();
		}
	 }
}

addLoadEvent(function() 
{ swapImage(); 
});

Sup3rkirby

The CODE tag is your friend.

Anyway, this is just my take on tidying up your code. I'm sure there are other ways to do it (some possibly better).

var $ = function (id){ return document.getElementById(id); }

var grpImages = ["images/blu.png",
"images/org.png",
"images/prp.png",
"images/ylw.png",
"images/grn.png",
"images/red.png"];

var grpCaptions = ["Blue Suede",
"Orange Peel",
"Purple Rain",
"Lemon Yellow",
"Kelly Green",
"Scarlet Red"];

var i = 0;
var j = grpImages.length-1;

var swapImage = function() {
	var imgCase = $("slide");
	//imgCase.src = image[i]; // need this line for IE 8
	imgCase.setAttribute( "src", grpImages[i] ); // comment out this line for IE8

var captionBox = $("myDiv");
captionBox.innerHTML = grpCaptions[i];

i = (i < j) ? ++i : 0;
setTimeout(swapImage, 2500);
}

function addLoadEvent(func){
  var oldonload = window.onload;
	window.onload = (typeof window.onload !== 'function') ? func : function() {	if(oldonload) oldonload(); func(); };
}

addLoadEvent(function(){ swapImage(); });

13thwarrior

I tried the revision and now it doesn't work. Not sure why, it looks correct. . . I'm taking a closer look at it now. . . Thanks.
. . .

Sup3rkirby

It could be that there needs to be a semi-colon after the $ and swapImage vars are declared.

13thwarrior

Fixed it. . . Here's how it looks finished.

addLoadEvent = function (func)
{ oldonload = window.onload;
window.onload = (typeof window.onload !== 'function') ? func : function ()
{ if (oldonload)
{ oldonload();
}
func();
};
};

addLoadEvent(function ()
{ swapImage(); 
});

I appreciate the help. . .

13thwarrior

Meant to say. . . I revised the script to work in 'strict' . . . so the variables are setup earlier in the script I'm actually using.

Thanks. . .