www.webdeveloper.com
Results 1 to 6 of 6

Thread: [RESOLVED] Tidy up javascript

  1. #1
    Join Date
    Aug 2008
    Posts
    40

    resolved [RESOLVED] Tidy up javascript

    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.
    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(); 
    });
    d
    Last edited by jedaisoul; 03-20-2014 at 03:44 PM. Reason: code tags added

  2. #2
    Join Date
    Mar 2005
    Location
    Behind you...
    Posts
    946
    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).
    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];
    
    	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(); });
    "Given billions of tries, could a spilled bottle of ink ever fall into the words of Shakespeare?"

  3. #3
    Join Date
    Aug 2008
    Posts
    40
    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.
    . . .

  4. #4
    Join Date
    Mar 2005
    Location
    Behind you...
    Posts
    946
    It could be that there needs to be a semi-colon after the $ and swapImage vars are declared.
    "Given billions of tries, could a spilled bottle of ink ever fall into the words of Shakespeare?"

  5. #5
    Join Date
    Aug 2008
    Posts
    40
    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. . .

  6. #6
    Join Date
    Aug 2008
    Posts
    40
    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. . .

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  
HTML5 Development Center



Recent Articles