Error in javascript recursive function

Question

I have the following piece of code (for the present case it may be considered as a function to remove the attributes from a valid html string fed as input):

function parse(htmlStr)
{
console.log(htmlStr);
result+="<"+htmlStr.tagName.toLowerCase()+">";
var nodes=htmlStr.childNodes;
for(i=0;i<nodes.length;i++) {
    var node=nodes[i];
    if(node.nodeType==3) {
        var text=$.trim(node.nodeValue);
        if(text!=="") {
            result+=text;
        }
    }
    else if(node.nodeType==1) {
        result+=parse(node);
    }
}
result+="</"+htmlStr.tagName.toLowerCase()+">";
return result;
}

But it is not working as expected. For example, in the following case when I feed it the following html as input:

<div id="t2">
    Hi I am
    <b>
      Test
    </b>
</div>

it returns <div>Hi I am<div>Hi I am<b>Test</b></div>.

Also the page crashes if some large input is given to the function.

NOTE: I know there are better implementations of removing attributes from a string using jQuery, but I need to work with the above function here & also the complete code is not for removing attributes, the above is just a shortened part of the code

@Vega: shouldn't it be just <div>Hi I am<b>Test</b></div>? — gopi1410
– gopi1410, Commented Jun 5, 2012 at 16:13
@Vega: even console.log(htmlStr) shows <div id="t2">...</div> once, so how come it gets printed twice?? — gopi1410
– gopi1410, Commented Jun 5, 2012 at 16:15

Bergi · Accepted Answer · 2012-06-05 16:29:35Z

4

There is something wrong with your result variable. It is undefined and global. In each recursion you would append the same string to itself, which also makes it crashing for huge inputs. (I can't reproduce anything, it crashes right away with a Undefined variable Error)

BTW: Your argument is no htmlStr, it is a domNode. And you're not parsing anything. Please don't use wrong self-documenting variable names.

Corrected version:

function serialize(domElement) {
    var tagname = domElement.tagName.toLowerCase();
    var result = "<"+tagname+">";
//  ^^^       ^ not a +=
    var children = domElement.childNodes;
    for (var i=0; i<children.length ;i++) {
//       ^^^ was also missing
         if (children[i].nodeType == 3) {
             result += children[i].data;
         } else if (children[i].nodeType == 1) {
             result += serialize(children[i]);
//                  ^^ add a child's result here
         }
    }
    result += "</"+tagname+">";
    return result;
}

I would not use trim(), that would produce <div>Hi<b>I</b>am</div> from <div>Hi <b>I</b> am</div>. You might do something like .replace(/\s+/g, " ").

edited Jun 5, 2012 at 16:29

answered Jun 5, 2012 at 16:22

Bergi

671k162 gold badges1k silver badges1.5k bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

gopi1410 Over a year ago

ohk, but any other alternative way to using a global variable?

gopi1410 Over a year ago

thanks for the last suggestion, using a regexp is a good option than trim.

Selvakumar Arumugam · Accepted Answer · 2012-06-05 16:23:15Z

3

This result+=parse(node); -> In you case you shouldn't merge the result inside recursion like that..

What happens is the return result from <b> recursion call appends the existing result with returned result. Where the existing result is <div>Hi I am and the returned result is <div>Hi I am<b>Test and so at the end of recursion you have <div>Hi I am<div>Hi I am<b>Test.

var result = '';
function parse(htmlStr) {        
    result += "<" + htmlStr.tagName.toLowerCase() + ">";    
    var nodes = htmlStr.childNodes;
    for (i = 0; i < nodes.length; i++) {
        var node = nodes[i];        
        if (node.nodeType == 3) {
            var text = $.trim(node.nodeValue);
            if (text !== "") {
                result += text;
            }
        } else if (node.nodeType == 1) {            
            parse(node);
        }
    }
    console.log(result);
    result += "</" + htmlStr.tagName.toLowerCase() + ">";
    return result;
}

Fixed fiddle: http://jsfiddle.net/FBnYT/

answered Jun 5, 2012 at 16:23

Selvakumar Arumugam

79.9k15 gold badges123 silver badges139 bronze badges

4 Comments

gopi1410 Over a year ago

if I append a <p>check<p> in the html, the page crashes! even the fiddle :P

Selvakumar Arumugam Over a year ago

@gopi1410 It is your recursion logic :P anyways.. let me check why it is failing.. most likely due to tail recursion.

gopi1410 Over a year ago

yeah, I know that :P but I had to use this only. Maybe as Vega said its due to the global variable.

Selvakumar Arumugam Over a year ago

@gopi1410 Nope.. not that.. but yea using local is suggested.. however adding another child node is leading to an endless recursion.

charlietfl · Accepted Answer · 2012-06-05 16:41:07Z

1

Change

result+="<"+htmlStr.tagName.toLowerCase()+">";

to:

var result="<"+htmlStr.tagName.toLowerCase()+">";

WOrks fine in demo: http://jsfiddle.net/qtuUA/

answered Jun 5, 2012 at 16:41

charlietfl

172k14 gold badges125 silver badges154 bronze badges

1 Comment

gopi1410 Over a year ago

Check appending a <p>check</p> in the html

veritasetratio · Accepted Answer · 2012-06-06 00:23:43Z

0

The crash occurs because the loop control variable is not locally scoped. So in addition to the other recommended changes:

for (var i = 0; i < nodes.length; i++)

...

answered Jun 6, 2012 at 0:23

veritasetratio

6114 silver badges6 bronze badges

Collectives™ on Stack Overflow

Error in javascript recursive function

4 Answers 4

2 Comments

4 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

4 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related