I have javascript code like below -

function initPage(){

    // Left Navigation Pane - moverOver effect:
    document.getElementById("imgHowToBuy").onmouseover = leftNavHoverIn;
    document.getElementById("imgNewAddition").onmouseover = leftNavHoverIn; 
    document.getElementById("imgMostPopular").onmouseover = leftNavHoverIn;
    document.getElementById("imgOffer").onmouseover = leftNavHoverIn;   
    document.getElementById("imgRecentlySold").onmouseover = leftNavHoverIn;
.....
}

Basically my code works (the function gets called and executes beautifully). But I dont think I am using the principles of best practice here. It looks a little wierd to keep calling the same function; can i somehow pass the id of the element as the argument, and then execute the function, so that this whole thing reduces to a single line of code?

I'm a self taught js guy :D

Thanks!

I have javascript code like below -

function initPage(){

    // Left Navigation Pane - moverOver effect:
    document.getElementById("imgHowToBuy").onmouseover = leftNavHoverIn;
    document.getElementById("imgNewAddition").onmouseover = leftNavHoverIn; 
    document.getElementById("imgMostPopular").onmouseover = leftNavHoverIn;
    document.getElementById("imgOffer").onmouseover = leftNavHoverIn;   
    document.getElementById("imgRecentlySold").onmouseover = leftNavHoverIn;
.....
}

Basically my code works (the function gets called and executes beautifully). But I dont think I am using the principles of best practice here. It looks a little wierd to keep calling the same function; can i somehow pass the id of the element as the argument, and then execute the function, so that this whole thing reduces to a single line of code?

I'm a self taught js guy :D

Thanks!

• javascript
• function
• dynamic
• arguments

• And what's your HTML mark-up? – David Thomas Commented Oct 14, 2011 at 12:49

4 Answers 4

You're not "calling the same function", you're simply assigning a single function to lots of elements on a page. I see nothing specifically wrong here.

You could store the id's of all the elements you want to assign this function to, which would reduce the number of lines of code, but there's no specific reason to do so.

var leftNavElements = ["imgHowToBuy","imgNewAddition"]; // etc....
for(var i=0;i<leftNavElements.length;i++){
    document.getElementById(leftNavElements[i]).onmouseover = leftNavHoverIn;
}

You could do something like this:

function assignMouseOver(elem) {
  document.getElementById(elem).onmouseover = leftNavHoverIn;
}

assignMouseOver("imgHowToBuy"); // etc

You could also pass in the element IDs as an array and loop through the array inside the function, which would reduce it to one line of (visible) code:

function assignMouseOver(elems) {
  if (elems.length>0) {
   for (var i=0; i<elems.length; i++) {
     document.getElementById(elems[i]).onmouseouver = leftNavHoverIn;
   }
  }
}

assignMouseOver(["imgHowtoBuy","etc","etc"]);

More checking / validation would be required for best practice but the above should help.

Try this:

var ids = ["imgHowToBuy", "imgNewAddition", "imgMostPopular", "imgOffer", "imgRecentlySold"]

var setMouseover = function (id) {
    document.getElementById(id).onmouseover = leftNavHoverIn;
};

for(var i=0;i<ids.length;i++) {
    setMouseover(ids[i]);
}

function doTheJob(id)
{
    document.getElementById(id).onmouseover = leftNavHoverIn
}

doTheJob("imgHowToBuy");
doTheJob("imgNewAddition");
doTheJob("imgMostPopular");
doTheJob("imgOffer");
doTheJob("imgRecentlySold");