Possible bug while implementing recursion

[kingquattro]

Hey,

     Either I am going complete nuts and not seing the most obvious mistake or there is really a bug, hope someone can help.  I have a herirachical xml document that I would like to flatten out.  I wrote a cfm code and it works perfectly, but when I put them logic in cfscript it does not work.   From what I can tell, when recursion returns back, and continues on, the old value in the loop is overwritten (counter value is not what it should be when the state was saved and recursion took place). I don't know how else to explain this.   Here is code in cfm and in cfscript, can you see something I am not? 

function flattenXML works just fine, but flattenXML2 does not.

[code]

<cfset xmlfile = "/xDocs/TAXONOMY_XMLDOC_131.xml" />

<cfset myDoc = xmlParse(xmlfile) />

<cfset theRootElement = myDoc.XmlRoot>

<cfdump var="#theRootElement.XMLChildren[1]#"/>

<cfset st = flatternXML2(theRootElement, "", structNew())/>

<cfdump var="#st#"/>

<cffunction name="flatternXML" access="private" returntype="struct">

<cfargument name="node" type="xml" required="true">

<cfargument name="str" type="string" required="true">

<cfargument name="lineage" type="struct" required="true" >

          <cfset t_name="NONE"/>

          <cfif structKeyExists(arguments.node.XmlAttributes, "NAME")>

                    <cfset t_name = arguments.node.XmlAttributes['NAME']/>

</cfif>

          <cfif len(arguments.str)>

                    <cfset arguments.str &= "; " & left(arguments.node.XmlName, 1) & "_" & t_name/> 

<cfelse>

                    <cfset arguments.str = left(arguments.node.XmlName, 1) & "_" & t_name/>

</cfif>

          <cfif ArrayLen(arguments.node.XmlChildren) eq 0>

                    <!---<cfoutput>#arguments.str#</cfoutput></br>--->

                    <cfset hash_key = hash(arguments.str, "MD5")/>

                    <cfif not structKeyExists(arguments.lineage, hash_key)>

                              <cfset structInsert(arguments.lineage, hash_key, arguments.str)/>

  <cfelse>

                              <cfoutput>duplicate lineage #arguments.str#<br/></cfoutput>

  </cfif>

                    <cfreturn arguments.lineage/>

</cfif>

<cfloop from="1" to="#arraylen(arguments.node.XmlChildren)#" index="i">

                    <cfset arguments.lineage = flatternXML(arguments.node.XmlChildren, arguments.str, arguments.lineage)/>

</cfloop>

          <cfreturn arguments.lineage/>

</cffunction>

<cffunction name="flatternXML2" access="private" returntype="struct">

<cfargument name="node" type="xml" required="true">

<cfargument name="str" type="string" required="true">

<cfargument name="lineage" type="struct" required="true" >

<cfscript>

                    //set name and prefix, of the current node

                    t_name = "NONE";

                    if (structKeyExists(arguments.node.XmlAttributes, "NAME"))

                              t_name = arguments.node.XmlAttributes['NAME'];

                    if (len(arguments.str))

                              arguments.str &= "; " & left(arguments.node.XmlName, 1) & "_" & t_name;

  else

                              arguments.str = left(arguments.node.XmlName, 1) & "_" & t_name;

                    //recursion end condition

                    if (arraylen(arguments.node.XmlChildren) eq 0) {

                              writeoutput(arguments.str & "</br>");

                              hash_key = hash(arguments.str, "MD5");

                              if (not structKeyExists(arguments.lineage, hash_key))

                                        structInsert(arguments.lineage, hash_key, arguments.str);

  else

                                        writeoutput("duplicate lineage: " & arguments.str & "<br/>");

                              return(arguments.lineage);

                    }

                    for(j=1; j lte arraylen(arguments.node.XmlChildren); j=j+1){

                              writeoutput("before " & j & "_" & arraylen(arguments.node.XmlChildren) & "<br/>");

                              arguments.lineage = flatternXML2(arguments.node.XmlChildren, arguments.str, arguments.lineage);

                              writeoutput("after " & j & "_" & arraylen(arguments.node.XmlChildren) & "<br/>");

                    }

                    return(arguments.lineage);

</cfscript>

</cffunction>

[/code]

[Adam Cameron. · Accepted solution]

At first glance (it's too much code to wade through thoroughly), you're not VARing any of your variables in your function. You must do this at the best of times for the sake of good practices and stability of code, but with recursive functionality it's essential.

--

Adam

[BKBK]

To help you with another pair of eyes, I will just translate the tag version into a script. You can then compare. Here it is:

<cfscript>

var t_name="NONE";

var output="";

var hash_key="";

var updatedStr="";

var updatedLineage=structNew();

var returnStruct=structNew();

updatedStr = arguments.str;

updatedLineage = arguments.lineage;

if (structKeyExists(arguments.node.XmlAttributes, "NAME")) {

    t_name = arguments.node.XmlAttributes['NAME'];

}

if (len(updatedStr)) {

    updatedStr &= "; " & left(arguments.node.XmlName, 1) & "_" & t_name;

}

else

    updatedStr = left(arguments.node.XmlName, 1) & "_" & t_name;

if (ArrayLen(arguments.node.XmlChildren) eq 0) {

     //output=output & updatedStr & "<br/>";

    hash_key = hash(updatedStr, "MD5");

    if (not structKeyExists(updatedLineage, hash_key)) {

        structInsert(updatedLineage, hash_key, updatedStr);

    }

    else

    //output=output & "duplicate lineage: " & updatedStr & "<br/>";

}

//returnStruct.output=output;

//returnStruct.updatedLineage=updatedLineage;

for (i=1; i LTE arraylen(arguments.node.XmlChildren); i=i+1) {

    updatedLineage = flatternXML(arguments.node.XmlChildren, updatedStr, updatedLineage);

    //updatedLineage = flatternXML(arguments.node.XmlChildren, updatedStr, updatedLineage).updatedLineage;

}

return updatedLineage;

// return returnStruct;

</cfscript>

It is good practice to leave variables in the arguments scope unmodified during the processing of a function. Modifying them increases complexity. This comes into play when you maintain the code later, as we are now doing here. The solution is obvious. Just define a new updatable variable. In this case, updatedStr and updatedLineage.

You will also notice I have commented out all display statements. It is bad practice to make a function do more than one thing at once. The function returns a variable. It should therefore not have the added burden of writing output. If you wish to return output plus some other result(s), then store them in a struct and return that.

[kingquattro]

Thanks Adam, BKBK. 

Adam you were right about VARing.  I have a nasty habbit of not VARing my variables, and <cfset...> does that, hence the reason why cfm code was wroking but not cfscript.  I have initiated all varaibles and function is producing expected results.

BKBK, the display statements were there as part of my debugging process.

Thanks a bunch.

[Adam Cameron.]

Adam you were right about VARing.  I have a nasty habbit of not VARing my variables, and <cfset...> does that, hence the reason why cfm code was wroking but not cfscript.

I don't get what you mean. <cfset> does nothing different that a script variable assignment. To VAR a function-location variables, one needs to specify the VAR keyword in either way.

EG:

<cfset var foo = "bar">

var foo = "bar";

(or just use the local scope as of CF9).

--

Adam

[BKBK]

kingquattro wrote:

BKBK, the display statements were there as part of my debugging process.

OK. Then please ignore the comment lines in my code sample and what I said about display.