Can I speed up this loop? 3

[Einstein47]

I have a question about speeding up the following loop. It is used to iterate through each element on a form and if it is of type "select-one" then it hides it. And it works but it is very slow when the form has a lot of elements. And what is most concerning is that it is still very slow even when the form has no "select-one" elements.

Is there a way to get only the "select-one" elements from the form and then loop through them? I'm thinking that would be faster, but I don't know if that is possible.

function PopupMenu_hideSelect() {
	for (j=0; j < window.top.window.frames["mainFrame"].document.forms.length; j++) {
		var theForm = window.top.window.frames["mainFrame"].document.forms[j];
		var numElems = theForm.elements.length;
		for(i=0; i < numElems; i++) {
			if(theForm.elements(i).type == "select-one") {
				theForm.elements(i).style.visibility = "hidden";
			}
		}
	}
}

Thanks in advance for any suggestions.

Einstein47
For best results: hand wash in cold, tumble dry low.
For not so good results: drag through puddles, pound on rocks, air dry on tree branch.
[[]Starbase47.com]

 

[feherke]

Hi

Probably. If you are interested only in [tt]select[/tt]s, then why to process the others ?

function PopupMenu_hideSelect()
{
    var selElems=top.mainFrame.document.getElementsByTagName('select');
    var numElems=selList.length;
    for(i=0; i < numElems; i++) {
         if(selElems[i].type == "select-one") {
            selElems[i].style.visibility = "hidden";
         }
    }
}

Or if you have no [tt]select[/tt] with [tt]multiple="multiple"[/tt] then even more simple :

function PopupMenu_hideSelect()
{
    var selElems=top.mainFrame.document.getElementsByTagName('select');
    var numElems=selList.length;
    for(i=0; i < numElems; i++) {
        selElems[i].style.visibility = "hidden";
    }
}

Feherke.

http://rootshell.be/~feherke/

 

[BabyJeffy]

I can't see any real way to improve on the code you've posted (maybe the outer loop would be better off testing the loop condition using a variable to prevent looking through the whole object each iteration - but this won't really help your scenario).

How about modifying the inner loop to use getElementsByTagName - this may change the speed of things:

function PopupMenu_hideSelect() {
    for (j=0; j < window.top.window.frames["mainFrame"].document.forms.length; j++) {
        var theForm = window.top.window.frames["mainFrame"].document.forms[j];
        var inputs = theForm.getElementsByTagName('INPUT');
        for(i=0, numInputs=inputs.length; i < numElems; i++) {
            if(theForm[i].type == "select-one") {
                theForm[i].style.visibility = "hidden";
            }
        }
    }
}

Let us know how you go!

Cheers,
Jeff

[tt]Jeff's Blog [!]@[/!] CodeRambler

http://www.coderambler.com/

http://www.coedit.co.uk/

[/tt]

Make sure your web page and css validates properly against the doctype you have chosen - before you attempt to debug a problem!

FAQ216-6094

 

[BabyJeffy]

*snap*

[tt]Jeff's Blog [!]@[/!] CodeRambler

http://www.coderambler.com/

http://www.coedit.co.uk/

[/tt]

Make sure your web page and css validates properly against the doctype you have chosen - before you attempt to debug a problem!

FAQ216-6094

 

[feherke]

Hi

*snap*

Was that related to this line ? ;-)

[tt]var inputs = theForm.getElementsByTagName('[red]INPUT[/red]');[/tt]

Feherke.

http://rootshell.be/~feherke/

 

[BabyJeffy]

Nods... and I saw you'd posted the same idea when I came to repost the correction

Cheers,
Jeff

[tt]Jeff's Blog [!]@[/!] CodeRambler

http://www.coderambler.com/

http://www.coedit.co.uk/

[/tt]

Make sure your web page and css validates properly against the doctype you have chosen - before you attempt to debug a problem!

FAQ216-6094

 

[Einstein47]

Thanks feherke, you did reduce the number of loops in my code, however this loop isn't what is slowing me down - darn. It must be in the conversion from the simple unordered list to the menu/sub-menus that is causing my delay.

Oh, and you had a typo in your example:

function PopupMenu_hideSelect()
{
    var selElems=top.mainFrame.document.getElementsByTagName('select');
    var numElems=[red]selElems[/red].length;
    for(i=0; i < numElems; i++) {
        selElems[i].style.visibility = "hidden";
    }
}

Einstein47
There are no kangaroos in Austria!
[[]Starbase47.com]

 

[Einstein47]

Would either of you be willing to help me find out how to speed up the formatting of my menu/submenus so that this delay isn't so noticeable?

I have taken the idea of using a multi-level unordered list building it dynamically as well as doing the formatting every time so that it displays correctly. Here is my code:

<script language="javascript" type="text/javascript">
var menuDiv;
var win;
[b]function [red]setInnerHTML(x,menu)[/red][/b] {
    if (menu.length > 0) {
      menuDiv = window.frames["mainFrame"].document.getElementById("menuDiv");
      if (win && menuDiv && menuDiv.style) {
		win.populate(menu);
		win.showPopup(x,0);
      }
	  else {
		var newElement = window.frames["mainFrame"].document.createElement("div");
		newElement.setAttribute("name", "menuDiv");
		newElement.setAttribute("id", "menuDiv");
		window.frames["mainFrame"].document.body.appendChild(newElement);
        with (window.frames["mainFrame"].document.getElementById("menuDiv").style) 
	{
		position = "absolute";
		backgroundColor = "transparent";
		color = "MenuText";
		width = "auto";
		margin = "0";
		padding = "0";
		fontFamily = "Arial";
		fontSize = "8pt";
		overflow = "visible";
	}
	if (document.all) {
	[teal]// This is for IE specific browsers[/teal]
	  with (window.frames["mainFrame"].document.styleSheets[0]) {
            addRule("#menuDiv ul","margin:0;padding:0;list-style-type:none;width:210px;border:2px outset ThreeDFace;background:Menu;");
            addRule("#menuDiv ul li","position: relative;background:Menu;overflow:visible;");
            addRule("#menuDiv ul li ul","position: absolute;width:155px;top:0;visibility:hidden;background:Menu;");
            addRule("#menuDiv ul li a","font-family:Arial;font-size: 8pt;display:block;overflow:auto;padding:1px 6px 1px 3px;color:MenuText;background-color:ThreeDFace;text-decoration:none;");
            addRule("#menuDiv a:hover","color:HighlightText;background-color:Highlight;");
            addRule("#menuDiv hr","padding:0;margin-top:-2px;color:ThreeDDarkShadow;");
            addRule("#menuDiv .subfolderstyle","padding-right:0;background: url(/site/ArrowPopup.gif) no-repeat center right;");
            addRule("* html #menuDiv ul li","float: left; height: 1%;");
            addRule("* html #menuDiv ul li a","height: 1%;");
	  }
	}
	else if (document.getElementById) {
	[teal]// This is for FireFox and Non-IE browsers[/teal]
            var x = window.frames["mainFrame"].document.styleSheets[0];
            x.insertRule("#menuDiv ul {margin:0;padding:0;list-style-type:none;width:210px;border:2px outset ThreeDFace;background:Menu;}",x.cssRules.length);
            x.insertRule("#menuDiv ul li {position: relative;background:Menu;overflow:visible;}",x.cssRules.length);
            x.insertRule("#menuDiv ul li ul {position: absolute;width:155px;top:0;visibility:hidden;background:Menu;}",x.cssRules.length);
            x.insertRule("#menuDiv ul li a {font-family:Arial;font-size: 8pt;display:block;overflow:auto;padding:1px 5px;color:MenuText;background-color:ThreeDFace;text-decoration:none;}",x.cssRules.length);
            x.insertRule("#menuDiv a:hover {color:HighlightText;background-color:Highlight;}",x.cssRules.length);
            x.insertRule("#menuDiv hr {margin-top:-2px;padding:0px;color:ThreeDDarkShadow;}",x.cssRules.length);
            x.insertRule("#menuDiv .subfolderstyle {background: url(/site/ArrowPopup.gif) no-repeat center right;}",x.cssRules.length);
            x.insertRule("* html #menuDiv ul li {float: left; height: 1%;}",x.cssRules.length);
            x.insertRule("* html #menuDiv ul li a {height: 1%;}",x.cssRules.length);
	}

		win = new PopupMenu('menuDiv');
		win.autoHide();
		if (!win.listenerAttached) {
		  win.listenerAttached = true;
		  win.attachListener();
		}
		win.populate(menu);
		win.showPopup(x,0);
	   menuDiv = window.frames["mainFrame"].document.getElementById("menuDiv");
	  }
	  [teal]// Run script to convert unordered list to menus[/teal]
	  [red][b]buildsubmenus();[/b][/red]
	  [teal]// if main menu is too wide and the right edge extends off the screen, move it left[/teal]
	  var clientWidth = parseInt(window.frames["mainFrame"].document.body.clientWidth,10);
	  var done = false;
	  var elemWidth = parseInt(menuDiv.offsetWidth,10);
	  while (((getLeftPos(menuDiv)+elemWidth) > clientWidth) && !done) {
		menuDiv.style.left = (getLeftPos(menuDiv) - 5) + "px";
		elemWidth = parseInt(menuDiv.offsetWidth,10);
	  }
    }
}

[teal]// This is called from clicking the link on the menu so that
// I can either create a popup window, or change the content
// of the mainFrame to be the requested page.[/teal]
[b]function [red]goMenu(url)[/red][/b] {
	var urlStr = "";
	if (url.charAt(0) == "p") {
		urlStr = "window.open('"+url.substr(1)
			+"','child','toolbar=no,width=700,height=500,resizable=yes,scrollbars=yes')";
		if (win && win.hidePopup) win.hidePopup();
	}
	else {
		window.frames["mainFrame"].document.body.style.pointer = 'wait';
		urlStr = "window.frames[\"mainFrame\"].document.location.href='" + url +"';";
		win = null;
		menuDiv = null;
	}
	eval(urlStr);
}

[teal]/* This function will shrink the width of the given DIV until it can no longer
   shrink, or until the height gets larger and then it will increase the size
   slightly. */[/teal]

[b]function [red]resizeMenu(menuDivObj)[/red][/b] {
[teal]// When submenus have no spaces, the width can only be reduced so far[/teal]
	var origHeight = menuDivObj.offsetHeight;
	var prevWidth = 0;
	while (origHeight == menuDivObj.offsetHeight
		&& menuDivObj.offsetWidth != prevWidth) {
		prevWidth = menuDivObj.offsetWidth;
		menuDivObj.style.width = (menuDivObj.offsetWidth-3)+"px";
	}
	menuDivObj.style.width = (menuDivObj.offsetWidth+6)+"px";
}
    
[b]function [red]getLeftPos(inputObj)[/red][/b] {
    var returnValue = inputObj.offsetLeft;
    while((inputObj = inputObj.offsetParent) != null)
    	returnValue += inputObj.offsetLeft;
    return parseInt(returnValue,10);
}
	
[teal]//SuckerTree Vertical Menu 1.1 (Nov 8th, 06)
//By Dynamic Drive: [URL unfurl="true"]http://www.dynamicdrive.com/style/[/URL][/teal]

var menuids=["suckermenu"] //Enter id(s) of SuckerTree UL menus, separated by commas

[b]function [red]buildsubmenus()[/red][/b] {
	var menuObj = window.frames["mainFrame"].document.getElementById(menuids[0]);
	resizeMenu(menuObj);
	var clientWidth = parseInt(window.frames["mainFrame"].document.body.clientWidth,10);
	for (var i=0; i<menuids.length; i++) {
		var ultags=window.frames["mainFrame"].document.getElementById(menuids[i]).getElementsByTagName("ul")
	    for (var t=0; t<ultags.length; t++){
			resizeMenu(ultags[t]);
			ultags[t].parentNode.getElementsByTagName("a")[0].className="subfolderstyle"
			var myLeft;
			//if this is a first level submenu
			if (ultags[t].parentNode.parentNode.id == menuids[i]) {
			//dynamically position first level submenus to the right of main menu item
				myLeft = ultags[t].parentNode
			}
			else { //else if this is a sub level submenu (ul)
			//position menu to the right of menu item that activated it
				var myLeft = ultags[t].parentNode.getElementsByTagName("a")[0];
			}
			var leftEdge = parseInt(getLeftPos(myLeft))+parseInt(myLeft.offsetWidth);
			var elemWidth = parseInt(ultags[t].offsetWidth);
			if ((leftEdge+elemWidth) < clientWidth)
				ultags[t].style.left=myLeft.offsetWidth+"px" 
			else {
			// the submenu needs to be placed to the right of the parent UL item
				ultags[t].style.left=(-elemWidth)+"px";
			}
		    ultags[t].parentNode.onmouseover=function(){
		    	this.getElementsByTagName("ul")[0].style.display="block"
				this.style.backgroundColor = "Highlight";
				this.style.color = "HighlightText";
		    }
		    ultags[t].parentNode.onmouseout=function(){
		    	this.getElementsByTagName("ul")[0].style.display="none"
				this.style.backgroundColor = "Menu";
				this.style.color = "MenuText";
		    }
    	}
		for (var t=ultags.length-1; t>-1; t--) { 
			//loop through all sub menus again, and use "display:none" to hide menus 
			//(to prevent possible page scrollbars)
			ultags[t].style.visibility="visible"
			ultags[t].style.display="none"
		}
	}
}

</script>

The slow part is the buildsubmenus() function. Would there be a way for me to maybe store the contents in an array and then just use that on repeated calls to the same menu? I just don't know if I can do something like .innerHTML to this dynamic element and then save that off.

Einstein47
There are no kangaroos in Austria!
[[]Starbase47.com]

 

[monksnake]

A useful quick way to speed up loops involves NOT using the .length in the loop itsself.

set the length of the loop to a variable, THEN use that in the loop.

Example:

var menuids=["suckermenu"] //Enter id(s) of SuckerTree UL menus, separated by commas

function buildsubmenus() {
    var menuObj = window.frames["mainFrame"].document.getElementById(menuids[0]);
    resizeMenu(menuObj);
    var clientWidth = parseInt(window.frames["mainFrame"].document.body.clientWidth,10);
    for (var i=0; i<[!]menuids.length[/!]; i++) {
        var ultags=window.frames["mainFrame"].document.getElementById(menuids[i]).getElementsByTagName("ul")
        for (var t=0; t<[!]ultags.length[/!]; t++){

What I have highlighted, set into variables.

var menusIDLength = menuids.length;
var ulTagLength = ultags.length;

Then use these variables in your for loops

It will speed it up enormously

<.

 

[Einstein47]

Thanks for the suggestion - I just tried that and I didn't notice a difference. I'm wondering if my [red]resizeMenu()[/red] function is where the slowdown happens. I'll play with the values that I am subtracting and see if that can speed things up.

I do appreciate the help. I'll lick this thing yet.

Einstein47
There are no kangaroos in Austria!
[&#91;]Starbase47.com]

 

[BabyJeffy]

I use this kind of syntax to save having to add another line:

    for (var i=0[!], max=menuids.length[/!]; i<[!]max[/!]; i++) {

I'll take a peek at the code as it is and give any feedback if I find something.

Cheers,
Jeff

[tt]Jeff's Blog [!]@[/!] CodeRambler

http://www.coderambler.com/

http://www.coedit.co.uk/

[/tt]

Make sure your web page and css validates properly against the doctype you have chosen - before you attempt to debug a problem!

FAQ216-6094

 

[monksnake]

BabyJeffy, syntax like this

for (var i=0, max=menuids.length; i<max; i++) {

can kill execution time, especially if menuids is a list of objects.

I never notice a difference on regular number or string arrays, but arrays of objects slow down a lot when referencing .length in the loop.

The only way I know this is that my computer is so sh!tty (350 mHz, Pentium II), that I constantly have to optimize script running time.


<.

 

[Einstein47]

Well this has made the delay more bearable, but the menus are a little wider than what I would like. I have changed the following amounts in the resizeMenus() function:

function resizeMenu(menuDivObj) {
// When submenus have no spaces, the width can only be reduced so far
    var origHeight = menuDivObj.offsetHeight;
    var prevWidth = 0;
    while (origHeight == menuDivObj.offsetHeight
        && menuDivObj.offsetWidth != prevWidth) {
        prevWidth = menuDivObj.offsetWidth;
        menuDivObj.style.width = (menuDivObj.offsetWidth-[red]10[/red])+"px";
    }
    menuDivObj.style.width = (menuDivObj.offsetWidth+[red]12[/red])+"px";
}

Basically this is reducing the number of times the while loop processes. However, by increasing the amounts the width of my menus and submenus is more than I would like. I guess it is a small price to pay.

Here is a question: Is there a speed difference between declaring variables with the [red]var[/red] keyword and without it? Maybe that might help my speed?

Einstein47
There are no kangaroos in Austria!
[&#91;]Starbase47.com]

 

[monksnake]

set some date variables in your code and put some alerts in it to find out which area specifically is running slowly.

var startTime = new Date();
code
...
...
code.
..
...
var endTime = new Date();
alert(endTime - startTime);

<.

 

[Einstein47]

This seems to help:

function resizeMenu(menuDivObj) {
// When submenus have no spaces, the width can only be reduced so far
var a = new Date();
	var origHeight = menuDivObj.offsetHeight;
	var prevWidth = 0;
	var delta = 0;
	while (origHeight == menuDivObj.offsetHeight
				&& menuDivObj.offsetWidth != prevWidth) {
		prevWidth = menuDivObj.offsetWidth;
		[red]delta = Math.ceil(menuDivObj.offsetWidth * 0.12)[/red];
		menuDivObj.style.width = (menuDivObj.offsetWidth-[red]delta[/red])+"px";
	}
	[blue]while (origHeight != menuDivObj.offsetHeight) {
		menuDivObj.style.width = (menuDivObj.offsetWidth+4)+"px";
	}[/blue]
	menuDivObj.style.width = (menuDivObj.offsetWidth+2)+"px";
var b = new Date();
timer += "\n resize: " + (b-a);
}

As you can see instead of having a fixed value that I subtract from the width each time, I use a percentage of the width each time. And then I do another loop to increase the width until the text stops wrapping (the height returns to the original size), and then I add 2 pixels for good measure.

Even with an extra loop and more calculations - the resize function is much faster.

Thanks monksnake for the suggestion of the timers to pinpoint the exact location of the slowness.

Einstein47
There are no kangaroos in Austria!
[&#91;]Starbase47.com]

 

[BabyJeffy]

Monksnake, you mean that declaring it in the initial part of the for loop is not the same as declaring it outside the for loop?

I had always considered these to be the same in terms of speed of execution:

var max = menuObj.length;
for (loop=0; loop<max; loop++) { ... }

And:

for (loop=0, max=menuObj.length; loop<max; loop++) { ... }

Since the variable max is set at the point the loop variable is set, it isn't having to reference the actual object set to calculate the length each time.

Or am I missing something?

Cheers,
Jeff

[tt]Jeff's Blog [!]@[/!] CodeRambler

http://www.coderambler.com/

http://www.coedit.co.uk/

[/tt]

Make sure your web page and css validates properly against the doctype you have chosen - before you attempt to debug a problem!

FAQ216-6094

 

[monksnake]

Monksnake, you mean that declaring it in the initial part of the for loop is not the same as declaring it outside the for loop?

No, not exactly.

When you reference a length like this:

document.getElementsByTagName('input').length

I'm not sure exactly what happens, but I believe that it has to build the objectArray everytime through the loop.


When you reference a length like this:

var objArrayLength = document.getElementsByTagName('input').length

You create a pointer reference to the value in memory, so it just directly accesses this value instead of calculating it each time.

This is very noticable on large loops.

<.