CODEWITHSUNDEEP
javascriptrecursion
Mike Thomsen
Mike Thomsen

Reputation: 37506

What am I doing wrong with this recursive JavaScript function?

Here is the entire page:

<html>
<head>
        <script type="text/javascript" src="/jquery.js"></script>
        <script type="text/javascript" src="/json2.js"></script>
        <script type="text/javascript">
                function buildList(point) {
                        if ( !point )
                                return [];
                        children = [];
                        lis = point.children('li');
                        for (index = 0; index < lis.length; index++) {
                                id = $(lis[index]).attr('id');
                                parts = id.split('-');
                                title = $(lis[index]).children('div').text();
                                newObj = {
                                        id: parts[1],
                                        mtype: parts[0],
                                        label: title
                                }
                                ol = $(lis[index]).children('ol');
                               // if (ol.length == 1) {
                               //         newObj['childobjects'] = buildList(ol);
                               // }
                                children.push(jQuery.extend(true, {}, newObj));
                        }
                        return children;
                }
                $(function() {
                        obj = buildList( $('#menu-top') );
                        alert( JSON.stringify(obj) );
                });
        </script>
</head>
<body>
<ol id="menu-top" class="sortable ui-sortable">
        <li id="entry-16608">
                <div>Test item 1</div>
                <ol>
                        <li id="entry-16607" ">
                                <div>News, links and random thoughts</div>
                        </li>
                </ol>
        </li>
        <li id="entry-16609">
                <div>How a data retention mandate will likely lead to de facto censorship in the US</div>
        </li>
        <li id="entry-16579">
                <div>Git cheat sheet</div>
        </li>
</ol>
</body>
</html>

When I comment out the recursive call, my JSON looks like this:

[
   {
      "id":"16608",
      "mtype":"entry",
      "label":"Test item 1"
   },
   {
      "id":"16609",
      "mtype":"entry",
      "label":"How a data retention mandate will likely lead to de facto censorship in the US"
   },
   {
      "id":"16579",
      "mtype":"entry",
      "label":"Git cheat sheet"
   }
]

When I uncomment the code, the JSON looks like this:

[
   {
      "id":"16607",
      "mtype":"entry",
      "label":"News, links and random thoughts"
   },
   {
      "id":"16607",
      "mtype":"entry",
      "label":"News, links and random thoughts"
   }
]

I'm guessing this is the result of ignorance on my part about the finer details of how JavaScript handles scoping and recursion, but I'm at a loss as to what to do here.

Upvotes: 0

Views: 749

Answers (2)

Felix Kling
Felix Kling

Reputation: 816324

Yes, it is a scope problem. You forgot to put var in front of all variables. This makes the variables global, meaning each function call has access to the same variables (and is overwriting them).

See the fixed version: http://jsfiddle.net/fkling/uYXYh/

You could further improve the code by making use of more jQuery methods:

function buildList(point) {
    if (!point) return [];
    var children = [];
    point.children('li').each(function() {
        var parts = this.id.split('-');
        var newObj = {
            id: parts[1],
            mtype: parts[0],
            label: $(this).children('div').text()
        };
        var $ol = $(this).children('ol');
        if ($ol.length == 1) {
            newObj['childobjects'] = buildList($ol);
        }
        children.push(jQuery.extend(true, {}, newObj)); // not sure why you are
                                                        // doing this instead of
                                                        // children.push(newObj)
    });
    return children;
}

DEMO

Upvotes: 4

roto
roto

Reputation: 677

It appears that index is being declared as a global variable, and is getting mutated on each call to buildList. I think that's your problem (but maybe you're doing that on purpose for something that I'm not seeing). Try changing your for statement to:

for(var index=0; index < lis.length; index++){
    // ...
}

Upvotes: 1

Related Questions