What am I doing wrong with this recursive JavaScript function?

Question

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.

Felix Kling · Accepted Answer · 2011-02-20 18:32:18Z

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

roto · Accepted Answer · 2011-02-20 18:23:08Z

1

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++){
    // ...
}

answered Feb 20, 2011 at 18:23

roto

6776 silver badges11 bronze badges

1 Comment

Mike Thomsen Over a year ago

Thank you soooo much. That was it. In fact, the mutation issue was pervasive in my code.

Collectives™ on Stack Overflow

What am I doing wrong with this recursive JavaScript function?

2 Answers 2

Comments

1 Comment

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Linked

Related