can't do recursive function

[samuelberthelot]

Hi,
I'm trying to write a recursive fucntion that takes as parameters a
html div and an id. I have to recurse through all the children and
sub-children of the div and find the one that matches the id.
I have the following but it doesn't work well (algorighm issue) :

var cn = null;
function getChildElement(parent, childID){
for (var i=0; i<parent.childNodes.length; i++){
cn = parent.childNodes;
if(cn.getAttribute){

if(cn.getAttribute("id").toLowerCase().indexOf(childID.toLowerCase()) >
-1 ){
return cn;
}else{
return getChildElement(parent.childNodes, childID);
}
}
}
return cn;
}


Can you help please ?

 

[Vincent van Beveren]

What goes wrong? Why are you doing this anyway, ID's in a page should be
unique, so document.getElementById() should always work. You can check
with element.parentNode whether its in the correct location:

function checkParent(elem, requiredParent) {
while (elem.parentNode != null) {
if (elem.parentNode == requiredParent) {
return true;
}
elem = elem.parentNode;
}
return false;
}

Anyway, just a thought. The alogirth, looks good to me. You can invoke
getChildElement(cn, childID) instead of parent.childNodes. Do you
know what goes wrong? does it give an error?

Vincent

 

[RobG]

Hi,
I'm trying to write a recursive fucntion that takes as parameters a
html div and an id. I have to recurse through all the children and
sub-children of the div and find the one that matches the id.
I have the following but it doesn't work well (algorighm issue) :

var cn = null;

You don't need cn as a global, you can keep it local.

function getChildElement(parent, childID){

var cn;

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

It is more efficient to get the length of childNodes once and use that,
there might be lots of them!

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

You could also use a while loop, the following goes backwards through
the nodes (which shouldn't be an issue here):

var i = parent.childNodes.length
while (i--) {

cn = parent.childNodes;
if(cn.getAttribute){

if(cn.getAttribute("id").toLowerCase().indexOf(childID.toLowerCase()) >
-1 ){

A slightly simpler test (to me at least) is:

if (cn.id && cn.id.toLowerCase() == childID.toLowerCase()){

return cn;

So far so good, this will traverse all the child nodes and return a
match if one is found.

}else{

The above if will return if true and the function ends, so there is no
need for 'else'.

return getChildElement(parent.childNodes, childID);

But here you get into trouble. Firstly, 'cn' already refers to
parent.childNodes so use it.

Next, as a speed optimisation, only call getChildElement if cn has
children:

if (cn.childNodes){


It should be quicker to test here than create a new recursive function
object and test it there.

Next, when you recursively call getChildElement, the result will be
returned to the spot it was called from. Your code returns whatever
the call returns immediately, without testing. Have getChildElement
return either an element, or something that evaluates to false. If you
get an element, it must be a match, so return it (and so on back up the
recursion chain...).

If you don't get an element, don't return, let the loop continue.
Something like:

var x = getChildElement(cn, childID);
if (x) return x;
}

}
}
}
return cn;

At this point you should return something that will evaluate to false
in an if test. You can explicitly return false if you like, but you
could do it by inference by returning nothing. That will result in the
function returning 'undefined' if no element is returned, and hence x
will be false in the test above - it depends on your coding standards
and preferences.

}

I get the feeling I'm doing your homework, so I'll leave pasting the
bits together to you.

 

[RobG]

RobG wrote:
[...]

It is more efficient to get the length of childNodes once and use that,
there might be lots of them!

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

Gosh, that's ugly...

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

[...]