Can you optimize this script?

C

Chris Martin

Background: Our main menu is built using a simple unordered list
containing other ULs. There is a table on a page that holds the subnav.
Because of certain circumstances, I need to build the subnav dynamically
on each page. I wrote this script in about 15 minutes because it was a
"scope-creep" kinda thing. Now that I have a bit of time, for education
reasons, I want to optimize this thing as much as possible.

Notes: DOM is a heaven send :). By my own admission, the variable names
could be a little more descriptive. "nav" is the master navigation.
"subNav" is a table row which holds the sub-navigation. This script works
good enough. But, I'm always interested in the opinions of those more
experienced than I.

Here's a shortened version of the main nav:

<ul id="nav">
<li>
<div>
<a href="/home/newstory.aspx" alt="">Home</a>
</div>
</li>
<li>
<div>
<a href="/about/default.aspx" alt="ABOUT ASBA">About Asba
</a>
</div>
<ul>
<li>
<a href="/about/missionvisionphilosophy.aspx">
Mission/Vision/Philosophy</a>
</li>
<li>
<a href="/about/board.aspx">Board of Directors</a>
</li>
</ul>
</li>
</ul>

Here's the subnav:

<table class="subnav" border="0">
<tr>
<td class="subparent" id="subNavHeader">Benefits</td>
<td width="15" rowspan="2">&nbsp;</td>

</tr>
<tr>
<td class="subbody" id="subNav">

</td>
</tr>
</table>

Here's the script:

function makeSubNav(){
var navs = document.getElementById('nav').getElementsByTagName('a');

var subNav = document.getElementById('subNav');
var path = formatPath( document.location.pathname );
var anchor, link, parentNodeName, listToPass, altText;
for(var i = 0; i < navs.length; i++){
anchor = navs;
link = formatPath( anchor.pathname );
listToPass = null;
if(path == link){
switch(anchor.parentNode.nodeName.toUpperCase()){
case "LI":
listToPass =
anchor.parentNode.parentNode.parentNode;
break;
case "DIV":
listToPass = anchor.parentNode.parentNode;

break;
}
break;
}
}
try{
altText = listToPass.getElementsByTagName('div')
[0].getElementsByTagName('a')[0].attributes.getNamedItem('alt').value;
}catch(err){
altText = '';
}
subNav.appendChild( createList(listToPass, altText, path) );
}

function formatPath(path){
if(path.charAt(0) != '/') path = '/' + path;
return path.toLowerCase();
}

function createList(ul, altText, currentPage){
document.getElementById('subNavHeader').innerHTML = altText;
var toReturn = document.createElement('ul');
var items = ul.getElementsByTagName('li');
var img, existingLink, existingPath, newItem, newLink, span;
for(var i = 0; i < items.length; i++){
existingLink = items.getElementsByTagName('a')[0];
existingPath = formatPath( existingLink.pathname );
newItem = document.createElement('li');
img = new Image();
img.height = 6;
img.width = 10;
img.border = 0;
img.src = "/Common/Images/LinkBullet.gif";
newItem.appendChild( img );
if(existingPath == currentPage){
toReturn.appendChild( document.createElement('br') );
newItem.className = "subcurrent";
span = document.createElement('span');
span.innerHTML = existingLink.innerHTML;
newItem.appendChild( span );
toReturn.appendChild( newItem );
toReturn.appendChild( document.createElement('br') );
}else{
newLink = document.createElement('a');
newLink.href = existingLink.href;
newLink.innerHTML = existingLink.innerHTML;
newItem.appendChild( newLink );
toReturn.appendChild( newItem );
}
}
return toReturn;
}

TIA

--
( )
)\ ) ) ( /(
(()/( ( /( ( ( ( )\())
/(_)))\()))\ ( )\))( ((_)\
(_)) (_))/((_) )\ ) ((_))\ _ ((_)
/ __|| |_ (_) _(_/( (()(_)| |/ /
\__ \| _| | || ' \))/ _` | | ' <
|___/ \__| |_||_||_| \__, | |_|\_\
|___/
 
S

StingK

Background: Our main menu is built using a simple unordered list
containing other ULs. There is a table on a page that holds the
subnav. Because of certain circumstances, I need to build the subnav
dynamically on each page. I wrote this script in about 15 minutes
because it was a "scope-creep" kinda thing. Now that I have a bit of
time, for education reasons, I want to optimize this thing as much as
possible.

Notes: DOM is a heaven send :). By my own admission, the variable
names could be a little more descriptive. "nav" is the master
navigation. "subNav" is a table row which holds the sub-navigation.
This script works good enough. But, I'm always interested in the
opinions of those more experienced than I.

Here's a shortened version of the main nav:

<ul id="nav">
<li>
<div>
<a href="/home/newstory.aspx" alt="">Home</a>
</div>
</li>
<li>
<div>
<a href="/about/default.aspx" alt="ABOUT ASBA">About
Asba
</a>
</div>
<ul>
<li>
<a href="/about/missionvisionphilosophy.aspx">
Mission/Vision/Philosophy</a>
</li>
<li>
<a href="/about/board.aspx">Board of Directors</a>
</li>
</ul>
</li>
</ul>

Here's the subnav:

<table class="subnav" border="0">
<tr>
<td class="subparent" id="subNavHeader">Benefits</td>
<td width="15" rowspan="2">&nbsp;</td>

</tr>
<tr>
<td class="subbody" id="subNav">

</td>
</tr>
</table>

Here's the script:

function makeSubNav(){
var navs =
document.getElementById('nav').getElementsByTagName('a');

var subNav = document.getElementById('subNav');
var path = formatPath( document.location.pathname );
var anchor, link, parentNodeName, listToPass, altText;
for(var i = 0; i < navs.length; i++){
anchor = navs;
link = formatPath( anchor.pathname );
listToPass = null;
if(path == link){
switch(anchor.parentNode.nodeName.toUpperCase()){
case "LI":
listToPass =
anchor.parentNode.parentNode.parentNode;
break;
case "DIV":
listToPass = anchor.parentNode.parentNode;


break;
}
break;
}
}
try{
altText = listToPass.getElementsByTagName('div')
[0].getElementsByTagName('a')[0].attributes.getNamedItem('alt').value;
}catch(err){
altText = '';
}
subNav.appendChild( createList(listToPass, altText, path) );
}

function formatPath(path){
if(path.charAt(0) != '/') path = '/' + path;
return path.toLowerCase();
}

function createList(ul, altText, currentPage){
document.getElementById('subNavHeader').innerHTML = altText;
var toReturn = document.createElement('ul');
var items = ul.getElementsByTagName('li');
var img, existingLink, existingPath, newItem, newLink, span;
for(var i = 0; i < items.length; i++){
existingLink = items.getElementsByTagName('a')[0];
existingPath = formatPath( existingLink.pathname );
newItem = document.createElement('li');
img = new Image();
img.height = 6;
img.width = 10;
img.border = 0;
img.src = "/Common/Images/LinkBullet.gif";
newItem.appendChild( img );
if(existingPath == currentPage){
toReturn.appendChild( document.createElement('br') );
newItem.className = "subcurrent";
span = document.createElement('span');
span.innerHTML = existingLink.innerHTML;
newItem.appendChild( span );
toReturn.appendChild( newItem );
toReturn.appendChild( document.createElement('br') );
}else{
newLink = document.createElement('a');
newLink.href = existingLink.href;
newLink.innerHTML = existingLink.innerHTML;
newItem.appendChild( newLink );
toReturn.appendChild( newItem );
}
}
return toReturn;
}

TIA


No Ideas?

--
( )
)\ ) ) ( /(
(()/( ( /( ( ( ( )\())
/(_)))\()))\ ( )\))( ((_)\
(_)) (_))/((_) )\ ) ((_))\ _ ((_)
/ __|| |_ (_) _(_/( (()(_)| |/ /
\__ \| _| | || ' \))/ _` | | ' <
|___/ \__| |_||_||_| \__, | |_|\_\
|___/
 
T

Thomas 'PointedEars' Lahn

StingK said:

The name of the author of the precursor is sufficient to follow the
discussion, while long attributions make them less legible. Please
configure your UA not to post such attribution novels by default.

Whitespace like spaces and line-breaks are displayed as spaces. In a
standards compliant DOM they count as text nodes. They should not occur
after open tags and before close tags.

An A element does not have an ALT attribute. Why should it, such only
makes sense if the value of that attribute could be used as *alternative*
display, as with images or other objects.

<ul id="nav">

The HTML 4.01 Specification recommends to avoid line-breaks within
A elements.
[...]
Here's the subnav:

<table class="subnav" border="0">

Tables should not serve layout purposes alone, instead they should
only serve the purpose of arranging tabular data and to mark up the
relations between them.

Especially DOM methods should be feature-tested before applied, in
its most simple form:

if (document.getElementById)
{
var navs = document.getElementById('nav');
if (navs.getElementsByTagName
&& (navs = navs.getElementsByTagName('a')))
{
// access navs
}

More sophisticated but requires JavaScript 1.1 or an ECMAScript 2+
language implementation:

var t;
if ((t = typeof document.getElementById) == "function"
|| (t == "object" && document.getElementById))
{
var navs = document.getElementById('nav');
if (((t = typeof navs.getElementsByTagName) == "function"
|| (t == "object" && navs.getElementsByTagName))
&& (navs = navs.getElementsByTagName('a')))
{
// access navs
}

See also <http://pointedears.de/scripts/test/whatami> and
isMethodType() in said:
[...]
var path = formatPath( document.location.pathname );

Although possible, at least I don't consider it good style
if a method is called before its definition is known.

formatPath() should be defined *in the source* prior to
makeSubNav(), although it is defined during runtime before
makeSubNav() is called and so calling the former is possible.

Those variables should be declared within the "for" loop, so that
if the loop is for some reason not executed, they are not declared.

It is more efficient to write

for (var i = 0, len = navs.length; i < len; i++)
{


That variable is indeed useful to prevent further unnecessary look-ups.

However, that variable is not necessary, see [1]

Could be omitted if [2] is used

[1]

if (path == formatPath(anchor.pathname))

Since `anchor.parentNode' is accessed afterwards, look-ups can be saved
here. toLowerCase() should be preferred over toUpperCase() for performance
reasons (words are for the most part lowercase, and lowercase strings can
be compressed better):

for (...)
{
// ...
var p = anchor.parentNode;
if (p)
{
switch (p.nodeName.toLowerCase())
{
case "li":
listToPass = p.parentNode.parentNode;
break;

case "div":
listToPass = p.parentNode;
break;
}
break;
}
// ...
}
}
try{
altText = listToPass.getElementsByTagName('div')
[0].getElementsByTagName('a')[0].attributes.getNamedItem('alt').value;

altText = listToPass.getElementsByTagName('div')[0]
.getElementsByTagName('a')[0].alt;

Note that try...catch requires an ECMAScript 3 compliant language
implementation.

[2]
subNav.appendChild(createList(listToPass || null, altText, path));

Are you sure? The path component of URIs is case-sensitive.

`innerHTML' is not part of any standard, don't mix standards compliant
references with proprietary ones (at least don't do that without
feature-testing prior). Use either

var o;
if (document.getElementById
&& (o = document.getElementById('subNavHeader'))
&& (typeof o.innerHTML != "undefined"))
{
o.innerHTML = altText;
}

or

var o;
if (document.getElementById
&& (o = document.getElementById('subNavHeader'))
{
var oSpan, oText;
if (o.firstChild
&& o.removeChild
&& o.createTextNode
&& (oText = o.createTextNode(altText)))
{
while (o.firstChild)
{
o.removeChild(o.firstChild);
}

o.appendChild(oText);
}
}

or the latter with the more sophisticated feature-testing method,
see above.

At least

if (ul)
{
// access ul
}

is necessary here, since `ul' can be `null'.

See above.
existingLink = items.getElementsByTagName('a')[0];
existingPath = formatPath( existingLink.pathname );
newItem = document.createElement('li');
img = new Image();
img.height = 6;
img.width = 10;
img.border = 0;
img.src = "/Common/Images/LinkBullet.gif";


if (newItem)
{

Don't. What if document.createElement('br') is for some reason
unsuccessful?

See above.

Indentation should not exceed 4 spaces. To keep
source code legible, I recommend 2 spaces.

You are welcome.
No Ideas?

No name? No FAQ?


PointedEars
 
J

John G Harris

Thomas 'PointedEars' said:
The name of the author of the precursor is sufficient to follow the
discussion, while long attributions make them less legible. Please
configure your UA not to post such attribution novels by default.
<snip>

The article ID identifies the message. The author's name often does not.

John
 
D

Dr John Stockton

JRS: In article <[email protected]>, dated Sat, 16 Oct
2004 16:58:33, seen in Thomas 'PointedEars'
Lahn said:
The name of the author of the precursor is sufficient to follow the
discussion, while long attributions make them less legible. Please
configure your UA not to post such attribution novels by default.

Ignore the child Lahn.

He wishes to apply standards once considered appropriate in a minor
local hierarchy, while ignoring all other recommendations including
those of the relevant Usefor WIP.

The attribution heading this article is Usefor-compliant, whereas that
of StingK contains acceptable elements incorrectly formatted;

In <[email protected]>,

is a correct way to present those elements. However, the date of the
previous article is also generally considered useful. In the case of
Lahn's article, it would have shown that he is once again answering an
ancient article.


Lahn could usefully have pointed out that the Usenet signature
convention is to have no more than four lines.

Lahn's own signatures are not Usenet-convention-compliant; they breach
it in a different manner. Evidently he considers that RFC1855 does not
apply to him.
 
T

Thomas 'PointedEars' Lahn

John said:
<snip>

The article ID identifies the message. The author's name often does not.

The precursor's message ID is the last message ID in the References header
of the followup. In a reasonable archive, at least this header will be
stored to link the postings of the discussion in a thread; a good archive
will provide all references as visible hyperlinks. There is exactly no
need to duplicate that data in the body of the message, also taking into
account that it in fact does not ease retrieval of the information
contained therein, on the contrary.


F'up2 PointedEars
 
R

Randy Webb

Thomas said:
John G Harris wrote:




The precursor's message ID is the last message ID in the References header
of the followup. In a reasonable archive, at least this header will be
stored to link the postings of the discussion in a thread; a good archive
will provide all references as visible hyperlinks. There is exactly no
need to duplicate that data in the body of the message, also taking into
account that it in fact does not ease retrieval of the information
contained therein, on the contrary.

Thats rubbish, and entirely irrelevant. You are assuming that headers
are visible and available to anyone that uses News and that is not true.
Learn the difference, stop babbling about attributions (which you fail
to realize the importance of), and move on.
F'up2 PointedEars

F'up set back where it should be. You are also assuming that the News
software used by people follows and honors the Follow-up header. I know
of one, that when used, is used by ~20% of web users that has neither
the ability to set that Follow-up nor to cross-post.
 
J

John G Harris

Randy Webb said:
Thomas 'PointedEars' Lahn wrote:


F'up set back where it should be. You are also assuming that the News
software used by people follows and honors the Follow-up header. I know
of one, that when used, is used by ~20% of web users that has neither
the ability to set that Follow-up nor to cross-post.

Oh, so that's what F'up2 means. I thought it was describing Pointed
Ears.

John
 
T

Thomas 'PointedEars' Lahn

It should be where it is on-topic which after my comment is certainly no
longer this newsgroup. I am not to blame about this further off-topic
discussion here, Randy; you are.
Oh, so that's what F'up2 means. I thought it was describing Pointed
Ears.

FYI: My nickname does not contain a space character.

Followup-To: poster (F'up2 poster; here abbreviated as "F'up2 PointedEars")
means that the author of the message (here: me, thus the abbreviation)
wishes to continue the current discussion, if still necessary, only via
private e-mail (see RFC 1036), in most cases, as here, because further
discussion would be off-topic and thus disturb readers of the newsgroup.

It is a header value honored by all reasonable NetNews user agents but
broken Outlook Express which also sends the message to the newsgroup
if replied to such a message. It is _not_ up to the software of the
reader but up to the software of the poster of the message to *set* it.
People who reply are in most cases allowed by their software to post
to the newsgroup anyway (although that is not very polite, given the
reasons for setting Followup-To: poster above); it is not a must but
merely a suggestion, yet a strong one. It has nothing to do with
cross-posting, although the Followup-To header may be used also with
cross-postings to suggest the appropriate group where followups
(replies) should go to (to be on-topic).

People who use software that is not able to set Followup-To poster or to
post to more than one group should use different software, because their
current software is not compliant with widely accepted Internet and Usenet
standards (the RFC mentioned above, for example); however, I doubt that
such software exists -- even Google Groups allows for crossposting, not
yet for Followup-To: poster or to a newsgroup (which makes Google Groups
not a viable alternative for posting although good for read-only research,
and Google posters often killfiled, i.e. Google postings filtered; however,
they could cut the Newsgroups header to avoid further crosspostings;
however, as to my experience most people who use Google groups have no
clue about Google interconnecting with the world-wide distributed Usenet,
they consider it a Web bulletin board instead).

Those are facts that poor Randy still does not understand although they have
been already explained to him many times before, not only by me. Instead,
he is continuously expressing his unchanged sparkling incompetence in
matters of Usenet by whining about the bad bad people out there who know
how to choose working software, know how to handle that software and know
about the workings of the used communication medium; I suggest to ignore
that.

F'up2 poster set again, please follow. EOD for me here.


PointedEars
 
R

Randy Webb

Thomas said:
John G Harris wrote:




It should be where it is on-topic which after my comment is certainly no
longer this newsgroup. I am not to blame about this further off-topic
discussion here, Randy; you are.

Yep. I accept it. I put it back in where it originated. The
problem with setting follow-up to you and you alone is two-fold:

1) You can not return the email so there wouldn't be much point in
emailing you to begin with.

2) If it becomes private, then you are limited to two opinions. That
would be mine and yours. Ask in News, Comment in News, get rebuked in
News. Its the way of News.
FYI: My nickname does not contain a space character.

Either way, his assumption was a correct one.
Followup-To: poster (F'up2 poster; here abbreviated as "F'up2 PointedEars")
means that the author of the message (here: me, thus the abbreviation)
wishes to continue the current discussion, if still necessary, only via
private e-mail (see RFC 1036), in most cases, as here, because further
discussion would be off-topic and thus disturb readers of the newsgroup.

Oh, and this comes from a person who continuosly replies to post that
are over 3 weeks old? Wow. I am impressed.
It is a header value honored by all reasonable NetNews user agents but
broken Outlook Express which also sends the message to the newsgroup
if replied to such a message. It is _not_ up to the software of the
reader but up to the software of the poster of the message to *set* it.

You are assuming, incorrectly, that all News software (other than OE)
allows it or even honors it. They do not. AOL's newsreader being one of
them. And considering the fact that you can *not* use an external reader
when reading News via AOL, it would be kind of hard to set follow-ups,
honor them, or even be allowed to see what they are. It doesn't give you
that option.
People who reply are in most cases allowed by their software to post
to the newsgroup anyway (although that is not very polite, given the
reasons for setting Followup-To: poster above); it is not a must but
merely a suggestion, yet a strong one. It has nothing to do with
cross-posting, although the Followup-To header may be used also with
cross-postings to suggest the appropriate group where followups
(replies) should go to (to be on-topic).

And I think its appropriate in comp.lang.javascript. If you don't like
it, don't reply.
People who use software that is not able to set Followup-To poster or to
post to more than one group should use different software, because their
current software is not compliant with widely accepted Internet and Usenet
standards (the RFC mentioned above, for example); however, I doubt that
such software exists -- even Google Groups allows for crossposting, not
yet for Followup-To: poster or to a newsgroup (which makes Google Groups
not a viable alternative for posting although good for read-only research,
and Google posters often killfiled, i.e. Google postings filtered; however,
they could cut the Newsgroups header to avoid further crosspostings;
however, as to my experience most people who use Google groups have no
clue about Google interconnecting with the world-wide distributed Usenet,
they consider it a Web bulletin board instead).

If you can tell me how to use an external reader when using AOL as the
ISP that doesn't allow it, you might have a point. Otherwise, you are
spinning your wheels, spouting garbage, and generally showing your lack
of intelligence about the web in general. It is not always up to the
user what software they are/aren't allowed to use based on the ISP.

Those are facts that poor Randy still does not understand although they have
been already explained to him many times before, not only by me.

Now, you have done what you normally do. You are babbling nonsense about
whats been explained to me, by someone other than you. Well, sorry to
disappoint you but what you have to say to me has about as much value to
me as a salt block in a rain storm. But to give you the benefit of the
doubt, prove what you are saying by giving reference to articles
*under a year old* that prove what you are saying.
Instead, he is continuously expressing his unchanged sparkling incompetence in
matters of Usenet by whining about the bad bad people out there who know
how to choose working software, know how to handle that software and know
about the workings of the used communication medium; I suggest to ignore
that.

If you weren't so ludicrously stupid, you wouldn't be as funny.

F'up2 poster set again, please follow. EOD for me here.

I did, but I also posted it back where *I* wanted it posted. If you
can't handle a public discussion, then bow out.
 
J

John G Harris

F'up2 poster set again, please follow. EOD for me here.

Once again my eyes auto-expanded this to F***ed up duo, where duo is
presumably referring to a pair of auricular appendages with a
discontinuous outline.

John

PS Sensible people write "Follow up set".
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,774
Messages
2,569,598
Members
45,152
Latest member
LorettaGur
Top