...

View Full Version : Don't make functions within a loop. error from JSLint



river226
08-30-2010, 04:08 AM
I am working on a site and in the process borrowed some js from some one to get dropdown menus to work, but after a while got reports from people who tested my site of some problems and decided to verify all my code, and i have fixed most bugs but the one listed in the title.


sfHover = function() {
var sfEls = document.getElementById("nav").getElementsByTagName("LI");
for (var i=0; i<sfEls.length; i++) {
sfEls[i].onmouseover=function() {
this.className+=" sfhover";
};
sfEls[i].onmouseout=function() {
this.className=this.className.replace(new RegExp(" sfhover\\b"), "");
};
}
};
if (window.attachEvent) {window.attachEvent("onload", sfHover);}


and i am verifying it on jslint (http://www.jslint.com/)

the error i get is:


Error:

Problem at line 6 character 10: Don't make functions within a loop.

};

Problem at line 9 character 10: Don't make functions within a loop.

};

Implied global: sfHover 1,12, document 2, window 12


any help will be appreciated, and I'm sure this isn't complex but i don't know much js and want to keep it the same.

Old Pedant
08-30-2010, 05:23 AM
Tell jslint to jump in the lake. That's a perfectly reasonable way of coding what is being done there.

It *is* true that you don't *need* to make those particular functions in a loop, but there are other similar functions that you would need to do just that way.,

You should be able to get rid of the other message by just changing

sfHover = function() {
to

function sfHover() {
though again I think jslint is just being anally retentive.

Oh, w.t.h. If you want to move the function-building out of the loop:


function navOver() { this.className += " sfhover"; }
function navOut() { this.className = this.className.replace(new RegExp(/ sfhover\b/, ""); }

function sfHover() {
var sfEls = document.getElementById("nav").getElementsByTagName("LI");
for (var i=0; i<sfEls.length; i++) {
sfEls[i].onmouseover = navOver;
sfEls[i].onmouseout = navOut;
}
}
if (window.attachEvent) {window.attachEvent("onload", sfHover);}

Just out of curiosity, what do you do if window.attachEvent is *NOT* defined? (i.e., if its MSIE.)

mrhoo
08-30-2010, 05:48 AM
// You need a better event handler, yours only works in IE.


window.handler= (function(){
if(window.addEventListener){
return function(who, what, fun, mod){
mod= !!mod;
who.addEventListener(what, fun, mod);
}
}
else if(window.attachEvent){
return function(who, what, fun){
who.attachEvent('on'+what, fun);
}
}
})()

// If you set the event to fire when it bubbles up,
// you only need one event handler for the whole list.


sfHover= function(e){
e= window.event? event: e;
var t= e.type.toLowerCase()== 'mouseover',
who= e.target || e.srcElement,
c= who.className.replace(/ *sfhover */,'');
if(who.tagName== 'LI'){
if(t) c= c? c+" sfhover": '';
this.className= c;
}
}



handler(window,'load', function(){
var pa= document.getElementById("nav");
handler(pa,'mouseover', sfhover);
handler(pa,'mouseout', sfhover);
});

Old Pedant
08-30-2010, 07:39 AM
Yeah, nice way, Hoo.

Do you agree, though, that jslint is being overly picky for no good purpose??

Dormilich
08-30-2010, 03:25 PM
// You need a better event handler, yours only works in IE.

that code is the Suckerfish Dropdown fix for IE to allow :hover on elements other than <a> (i.e. <li>). It doesn’t need to run in other browsers.

rnd me
08-30-2010, 10:00 PM
functions in a loop are the only way to close certain values.
JSLint is good for corralling noobs to stop writing old-fashioned code styles (c, java, etc) but it has no place in a serious development workflow.

even crappy web pages validate; doesn't amount to a hill of beans for anyone beside the coder and a supervisor...

Old Pedant
08-30-2010, 10:52 PM
Thank you, RndMe, for confirming my assertions.

river226
09-07-2010, 12:25 AM
Thanks guys, and I will be the first to admit i am a noob with web programming, especially when it comes to js. just know that when i have goon to css forums for help there they have asked me to validate the code. but i seem to have it working, so thanks for your help.



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum