Don't make functions within a loop. error from JSLint

[river226]

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

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]

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]

// 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]

Yeah, nice way, Hoo.

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

[Dormilich]

Originally Posted by mrhoo

// 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]

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]

Thank you, RndMe, for confirming my assertions.

[river226]

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.