can't do recursive function

Discussion in 'Javascript' started by samuelberthelot@googlemail.com, Jul 11, 2006.

  1. Guest

    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 ?
    , Jul 11, 2006
    #1
    1. Advertising

  2. 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
    Vincent van Beveren, Jul 11, 2006
    #2
    1. Advertising

  3. RobG Guest

    wrote:
    > 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. :)

    --
    Rob
    RobG, Jul 12, 2006
    #3
  4. RobG Guest

    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++){

    [...]

    --
    Rob
    RobG, Jul 12, 2006
    #4
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. =?ISO-8859-1?Q?Gregory_Pi=F1ero?=

    Recursive function going infinite and I can't see why.

    =?ISO-8859-1?Q?Gregory_Pi=F1ero?=, Feb 4, 2006, in forum: Python
    Replies:
    4
    Views:
    317
    =?ISO-8859-1?Q?Gregory_Pi=F1ero?=
    Feb 5, 2006
  2. n00m
    Replies:
    12
    Views:
    1,111
  3. vamsi
    Replies:
    21
    Views:
    2,068
    Keith Thompson
    Mar 9, 2009
  4. Mark Piffer
    Replies:
    9
    Views:
    905
    luserXtrog
    May 15, 2009
  5. Alok
    Replies:
    3
    Views:
    246
Loading...

Share This Page