Opinions on function (recursive traversing DOM-tree)

[Rik]

Hi all,

I usually don't really use javascript, but for a pretty big form, I'm
trying the following:

I've got arbitrarily deep nested unorderd list, with in every <li3
checkboxes. It's for granting users certain rights, and rights are
inherited (or unset) downwards.

To directly illustrate what certain changes will do for the user, I want to
check/uncheck the particular checkboxes. I've got it working now, but it
seems to take longer then needed. Has anybody an idea how the following
could be done more effectively? (And working in at least MSIE, FF, Opera &
Netscape):

Online small example: http://www.rwasmus.nl/test.html

----HTML--------
<ul>
<li>
<input type="checkbox" class="small add"
onClick="changedown(this.className,this.parentNode ,this.checked);" />
<input type="checkbox" class="small edit"
onClick="changedown(this.className,this.parentNode ,this.checked);" />
<input type="checkbox" class="small del"
onClick="changedown(this.className,this.parentNode ,this.checked);" />
<ul>
<li><input type="checkbox" class="small add"
onClick="changedown(this.className,this.parentNode ,this.checked);" />
<input type="checkbox" class="small edit"
onClick="changedown(this.className,this.parentNode ,this.checked);" />
<input type="checkbox" class="small del"
onClick="changedown(this.className,this.parentNode ,this.checked);" />
</li>
</ul>
</li>
</ul>

--js function-----
function changedown(rtype,elid,rvalue){
var el = elid;
var i;
for(i=0; i < el.childNodes.length;i++){
var node = el.childNodes[i];
if(node.nodeName=='ul' || node.nodeName=='UL' || node.nodeName=='li' ||
node.nodeName=='LI'){
changedown(rtype,node,rvalue);
}
if((node.nodeName=='input' || node.nodeName=='INPUT' ) &&
node.className==rtype){
node.checked = rvalue;
}
}
}

TIA,
--
Rik Wasmus

[Martin Honnen]

Rik wrote:

Has anybody an idea how the following
could be done more effectively? (And working in at least MSIE, FF, Opera &
Netscape):

<ul>
<li>
<input type="checkbox" class="small add"
onClick="changedown(this.className,this.parentNode ,this.checked);" />

General comment, not meant to make your function more effective: if you
use /then you seem to be coding XHTML. Element and attribute names in
XHTML are all lower case so you should use onclick and not onClick. Of
course that does only matter if you serve the document with an XML or
XHTML MIME type like application/xml or application/xhtml+xml so that
the browser parses with an XML parser.

function changedown(rtype,elid,rvalue){
var el = elid;

That extra variable does not seem to be needed so simply doing
function changedown (rtype, el, rvalue) {
should suffice if you are looking for effective coding.

var i;
for(i=0; i < el.childNodes.length;i++){

var i, childNodes = el.childNodes, l = childNodes.length;
for (i = 0; i < l; i++) {

var node = el.childNodes[i];

var node = childNodes[i];
if (node.nodeType === 1) {
var tagName = node.tagName.toLowerCase();

if(node.nodeName=='ul' || node.nodeName=='UL' || node.nodeName=='li' ||
node.nodeName=='LI'){

if (tagName === 'ul' || tagName === 'li') {
changedown(rtype,node,rvalue);
}

changedown(rtype,node,rvalue);
}
if((node.nodeName=='input' || node.nodeName=='INPUT' ) &&
node.className==rtype){
node.checked = rvalue;
}

else if (tagName === 'input' && node.className === rtype) {
node.checked = true;
}
}
}

But alternatively it might be easier to simply use
getElementsByTagName('input') e.g.

function changedown (className, element, checked) {
var inputs = element.getElementsByTagName('input'), l = inputs.length;
for (var i = 0; i < l; i++) {
var input = inputs[i];
if (input.type.toLowerCase() === 'checkbox' && input.className
=== className) {
input.checked = checked;
}
}
}

If you know that the className comparison is sufficient then you don't
need the input.type to 'checkbox' comparison.
--

Martin Honnen
http://JavaScript.FAQTs.com/

[Rik]

Martin Honnen wrote:

Rik wrote:

>Has anybody an idea how the following
could be done more effectively? (And working in at least MSIE, FF,
Opera & Netscape):

><ul>
<li>
<input type="checkbox" class="small add"
onClick="changedown(this.className,this.parentNod e,this.checked);" />

General comment, not meant to make your function more effective: if
you
use /then you seem to be coding XHTML. Element and attribute names
in XHTML are all lower case so you should use onclick and not
onClick.

You're offcourse right. Normally I use XHTML, but flung together a HTML
document here for test-purposes. And on debugging I thought about changing
onclick to onClick because something didn't work. Created a bit of a mess
indeed.

Of course that does only matter if you serve the document
with an XML or
XHTML MIME type like application/xml or application/xhtml+xml so that
the browser parses with an XML parser.

Which I usually do (if the browser sends a correct HTTP_ACCEPT for XHTML).

>function changedown(rtype,elid,rvalue){
var el = elid;

That extra variable does not seem to be needed so simply doing
function changedown (rtype, el, rvalue) {
should suffice if you are looking for effective coding.

Yup, inheritance from another issue: I had an issue with scope, turned out
to be in the i though.

> var i;
for(i=0; i < el.childNodes.length;i++){

var i, childNodes = el.childNodes, l = childNodes.length;

Why not:
l = el.childNodes.length?
And if so, why not directly in the for loop? saves an otherwise useless
variable.

for (i = 0; i < l; i++) {

> var node = el.childNodes[i];

var node = childNodes[i];
if (node.nodeType === 1) {
var tagName = node.tagName.toLowerCase();

Good idea.

> if(node.nodeName=='ul' || node.nodeName=='UL' ||
node.nodeName=='li' || node.nodeName=='LI'){

if (tagName === 'ul' || tagName === 'li') {
changedown(rtype,node,rvalue);
}

> changedown(rtype,node,rvalue);
}
if((node.nodeName=='input' || node.nodeName=='INPUT' ) &&
node.className==rtype){
node.checked = rvalue;
}

else if (tagName === 'input' && node.className === rtype) {
node.checked = true;

Let's not do that, that will not uncheck it on an uncheck higher up :-).

}
}
}

But alternatively it might be easier to simply use
getElementsByTagName('input') e.g.

function changedown (className, element, checked) {
var inputs = element.getElementsByTagName('input'), l =
inputs.length; for (var i = 0; i < l; i++) {
var input = inputs[i];
if (input.type.toLowerCase() === 'checkbox' && input.className
=== className) {
input.checked = checked;
}
}
}

Which is unfortunately not the case, because the hiërarchie in the listing
matters a great deal. Unless you know a quick way to adress all input
elements directly descendant of a specific <li>.

If you know that the className comparison is sufficient then you don't
need the input.type to 'checkbox' comparison.

Which would normally be so, but as this is a dynamic form it is not totally
reliable.

But thanks for pointing out some sloppy code indeed :-)

Grtz,
--
Rik Wasmus

[Martin Honnen]

Rik wrote:

Martin Honnen wrote:

>>But alternatively it might be easier to simply use
getElementsByTagName('input') e.g.

function changedown (className, element, checked) {
var inputs = element.getElementsByTagName('input'), l =
inputs.length; for (var i = 0; i < l; i++) {
var input = inputs[i];
if (input.type.toLowerCase() === 'checkbox' && input.className
=== className) {
input.checked = checked;
}
}
}

Which is unfortunately not the case, because the hiërarchie in the listing
matters a great deal. Unless you know a quick way to adress all input
elements directly descendant of a specific <li>.

I don't understand your comment, you pass in an element node as the
second argument to that function and then my suggestion calls
element.getElementsByTagName('input') which does exactly what you are
asking for, namely "a quick way to adress all input elements directly
descendant" of that element passed in.
--

Martin Honnen
http://JavaScript.FAQTs.com/

[Martin Honnen]

Rik wrote:

Martin Honnen wrote:

>>var i;
for(i=0; i < el.childNodes.length;i++){

var i, childNodes = el.childNodes, l = childNodes.length;

Why not:
l = el.childNodes.length?
And if so, why not directly in the for loop? saves an otherwise useless
variable.

Simply to save the access to el.childNodes and el.childNodes.length that
your loop construct does every time your condition
i < el.childNodes.length
is evaluated. And childNodes is not useless, my suggestion uses it later
in the loop with
var node = childNodes[i];
--

Martin Honnen
http://JavaScript.FAQTs.com/

[Rik]

Martin Honnen wrote:

Rik wrote:

>Martin Honnen wrote:

>>>var i;
for(i=0; i < el.childNodes.length;i++){

var i, childNodes = el.childNodes, l = childNodes.length;

Why not:
l = el.childNodes.length?
And if so, why not directly in the for loop? saves an otherwise
useless variable.

Simply to save the access to el.childNodes and el.childNodes.length
that your loop construct does every time your condition
i < el.childNodes.length
is evaluated. And childNodes is not useless, my suggestion uses it
later in the loop with
var node = childNodes[i];

Once again, you're right :-). I didn't get that the first go, thanks for
your explanation.

Grtz,
--
Rik Wasmus

[Rik]

Martin Honnen wrote:

Rik wrote:

>Martin Honnen wrote:

>>But alternatively it might be easier to simply use
getElementsByTagName('input') e.g.

function changedown (className, element, checked) {
var inputs = element.getElementsByTagName('input'), l =
inputs.length; for (var i = 0; i < l; i++) {
var input = inputs[i];
if (input.type.toLowerCase() === 'checkbox' && input.className
=== className) {
input.checked = checked;
}
}
}

Which is unfortunately not the case, because the hiërarchie in the
listing matters a great deal. Unless you know a quick way to adress
all input elements directly descendant of a specific <li>.

I don't understand your comment,

Because my comment was nonsense..... IAW your right
--
Rik Wasmus

[Dr John Stockton]

JRS: In article <94***************************@news1.tudelft.nl> , dated
Mon, 4 Sep 2006 17:41:30 remote, seen in news:comp.lang.javascript, Rik
<lu************@hotmail.composted :

>
----HTML--------
<ul>
<li>
<input type="checkbox" class="small add"
onClick="changedown(this.className,this.parentNod e,this.checked);" />
<input type="checkbox" class="small edit"
onClick="changedown(this.className,this.parentNod e,this.checked);" />
<input type="checkbox" class="small del"
onClick="changedown(this.className,this.parentNod e,this.checked);" />
<ul>
<li><input type="checkbox" class="small add"
onClick="changedown(this.className,this.parentNod e,this.checked);" />
<input type="checkbox" class="small edit"
onClick="changedown(this.className,this.parentNod e,this.checked);" />
<input type="checkbox" class="small del"
onClick="changedown(this.className,this.parentNod e,this.checked);" />
</li>
</ul>
</li>
</ul>

Try calling changedown(this) above and picking out the elements in
the function below; if it works, it will at least shorten the code. We
don't like questioners who allow their posting agents to line-wrap
anything other than text.

>--js function-----
function changedown(rtype,elid,rvalue){
var el = elid;
var i;
for(i=0; i < el.childNodes.length;i++){

A while loop in the reverse order will be very slightly faster, even if
the for loop is improved by calculating the limit only once.

var J = el.childNodes.length ; while (J--) { ... }

var node = el.childNodes[i];
if(node.nodeName=='ul' || node.nodeName=='UL' || node.nodeName=='li' ||
node.nodeName=='LI'){
changedown(rtype,node,rvalue);
}

Look up nodeName once, into a temporary.
Precede those 4 tests with a check on nodeName.length=2, unless nearly
all nodeNames are length 2. Re-order the tests to put LI before UL, and
have the more probable capitalisation first.

If most nodeNames of length 2 are going to be 'list' ones, then TRY
getting the uppercase of nodeName and coding only two comparisons; it
*could* be faster.

if((node.nodeName=='input' || node.nodeName=='INPUT' ) &&
node.className==rtype){
node.checked = rvalue;

Likewise. And if the className test is more likely to fail, test it
first.

}
}
}

Consider whether you always terminate recursion as early as possible
(but probably you have no option).

--
© John Stockton, Surrey, UK. ?@merlyn.demon.co.uk Turnpike v4.00 IE 4 ©
<URL:http://www.jibbering.com/faq/>? JL/RC: FAQ of news:comp.lang.javascript
<URL:http://www.merlyn.demon.co.uk/js-index.htmjscr maths, dates, sources.
<URL:http://www.merlyn.demon.co.uk/TP/BP/Delphi/jscr/&c, FAQ items, links.

[Jim]

Martin Honnen wrote:

>

Rik wrote:

Simply to save the access to el.childNodes and el.childNodes.length that
your loop construct does every time your condition
i < el.childNodes.length
is evaluated. And childNodes is not useless, my suggestion uses it later
in the loop with
var node = childNodes[i];

I've seen similar suggestions in the past and back when I started
programming (FORTRAN, 1965) that was the case, however modern compilers
optimize these inefficiencies. Is javascript truly a 100% interpreted
language with no optimization? I'm not trying to be critical, I just am
not familiar with the internals of browsers.

Thanks,
Jim.

[Richard Cornford]

Jim wrote:

Martin Honnen wrote:

>Rik wrote:
Simply to save the access to el.childNodes and el.childNodes.length
that your loop construct does every time your condition
i < el.childNodes.length
is evaluated. And childNodes is not useless, my suggestion uses it
later in the loop with
var node = childNodes[i];

I've seen similar suggestions in the past and back when I started
programming (FORTRAN, 1965) that was the case, however modern
compilers optimize these inefficiencies. Is javascript truly a 100%
interpreted language with no optimization?

<snip>

It is not a matter of 100% interpreted or not, it is mostly a matter of
the dynamic and loosely typed nature of javascript. Javascript cannot
easily optimise the look-up in the loop because it cannot be certain
that the value referred to by - el.childNodes - is an object, let alone
an object with a - length - property, or the same object on each
iteration of the loop, or (even if the same object) an object with a
length property that will have the same value on each iteration of the
loop.

And even though some of these things could be deduced from an
examination of the contents of the loop the extra effort in testing for
those conditions is likely to count against any benefits gained from
any optimisation as those tests then need to be applied each time the
script is loaded (as opposed to being only applied once with a compiled
language).

Richard.

[Dr John Stockton]

JRS: In article <Yd******************@bignews3.bellsouth.net>, dated
Wed, 6 Sep 2006 05:09:12 remote, seen in news:comp.lang.javascript, Jim
<k4***@bellsouth.netposted :

>
I've seen similar suggestions in the past and back when I started
programming (FORTRAN, 1965) that was the case, however modern compilers
optimize these inefficiencies. Is javascript truly a 100% interpreted
language with no optimization? I'm not trying to be critical, I just am
not familiar with the internals of browsers.

Not 100%, at least because RegExp has a compile method.
But trade-offs differ. Delphi & C are intended to be run many times
after a final compilation; Javascript is distributed as written.

And, wherever a coder is aware of such things, it's rather easy for a
thoughtful coder to do a good fraction of possible optimisations, and
produce shorter and more readable code at the same time.
..
--
© John Stockton, Surrey, UK. ?@merlyn.demon.co.uk Turnpike v4.00 IE 4 ©
<URL:http://www.jibbering.com/faq/>? JL/RC: FAQ of news:comp.lang.javascript
<URL:http://www.merlyn.demon.co.uk/js-index.htmjscr maths, dates, sources.
<URL:http://www.merlyn.demon.co.uk/TP/BP/Delphi/jscr/&c, FAQ items, links.