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.

Answers:

Answer

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

Answer

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

Tags

Recent Questions

Top Questions

Home Tags Terms of Service Privacy Policy DMCA Contact Us Javascript

©2020 All rights reserved.