What's wrong with this ASP recursive function?

Question

When I call this function, everything works, as long as I don't try to recursively call the function again. In other words if I uncomment the line:

GetChilds rsData("AcctID"), intLevel + 1

Then the function breaks.

<%
    Function GetChilds(ParentID, intLevel)
     Set rsData= Server.CreateObject("ADODB.Recordset")
     sSQL = "SELECT AcctID, ParentID FROM Accounts WHERE ParentID='" & ParentID &"'"
     rsData.Open sSQL, conDB, adOpenKeyset, adLockOptimistic
     If IsRSEmpty(rsData) Then
      Response.Write("Empty")
     Else
      Do Until rsData.EOF
       Response.Write rsData("AcctID") & "<br />"
       'GetChilds rsData("AcctID"), intLevel + 1 
       rsData.MoveNext
      Loop
     End If
     rsData.close: set rsData = nothing
    End Function

    Call GetChilds(1,0)
%>

*Edited after feedback

Thanks everyone,

Other than the usual error:

Error Type: (0x80020009) Exception occurred.

I wasn't sure what was causing the problems. I understand that is probably due to a couple of factors.

1. Not closing the connection and attempting to re-open the same connection.
2. To many concurrent connections to the database.

The database content is as follows:

AcctID | ParentID
1        Null
2        1
3        1
4        2
5        2
6        3
7        4

The idea is so that I can have a Master Account with Child Accounts, and those Child Accounts can have Child Accounts of their Own. Eventually there will be Another Master Account with a ParentID of Null that will have childs of its own. With that in mind, am I going about this the correct way?

Thanks for the quick responses.

Answer 1

try declaring the variables as local using a DIM statement within the function definition:

Function GetChilds(ParentID, intLevel)
Dim rsData, sSQL
Set ...

Edit: Ok, I try to be more explicit.

My understanding is that since rsData is not declared by DIM, it is not a local variable, but a global var. Therefore, if you loop through the WHILE statement, you reach the .Eof of the inner-most rsData recordset. You return from the recursive function call, and the next step is again a rsData.MoveNext, which fails.

Please correct me if rsData is indeed local.

Answer 2

Running out of SQL Connections?

You are dealing with so many layers there (Response.Write for the client, the ASP for the server, and the database) that its not surprising that there are problems.

Perhaps you can post some details about the error?

Answer 3

How does it break?

My guess is that after a certain number of recursions you're probably getting a Stack Overflow (ironic) because you're not allocating too many RecordSets.

Answer 4

hard to tell without more description of how it breaks, but you are not using intLevel for anything.

Answer 5

Look like it fails because your connection is still busy serving the RecordSet from the previous call.

One option is to use a fresh connection for each call. The danger there is that you'll quickly run out of connections if you recurse too many times.

Another option is to read the contents of each RecordSet into a disconnected collection: (Dictionary, Array, etc) so you can close the connection right away. Then iterate over the disconnected collection.

If you're using SQL Server 2005 or later there's an even better option. You can use a CTE (common table expression) to write a recursive sql query. Then you can move everything to the database and you only need to execute one query.

Some other notes:
ID fields are normally ints, so you shouldn't encase them in ' characters in the sql string.

Finally, this code is probably okay because I doubt the user is allowed to input an id number directly. However, the dynamic sql technique used is very dangerous and should generally be avoided. Use query parameters instead to prevent sql injection.

I'm not too worried about not using intLevel for anything. Looking at the code this is obviously an early version, and intLevel can be used later to determine something like indentation or the class name used when styling an element.

Answer 6

In each call you open a new connection to the database and you don't close it before opening a new one.

Answer 7

Not that this is actually a solution to the recursion issue, but it might be better for you to work out an SQL statement that returns all the information in a hierarchical format, rather than making recursive calls to your database.

Come to think of it though, it may be because you have too many concurrent db connections. You continually open, but aren't going to start closing until your pulling out of your recursive loop.

Answer 8

Thanks everyone,

Other than the usual error:

Error Type: (0x80020009) Exception occurred.

I wasn't sure what was causing the problems. I understand that is probably due to a couple of factors.

1. Not closing the connection and attempting to re-open the same connection.
2. To many concurrent connections to the database.

The database content is as follows:

AcctID | ParentID
1        Null
2        1
3        1
4        2
5        2
6        3
7        4

The idea is so that I can have a Master Account with Child Accounts, and those Child Accounts can have Child Accounts of their Own. Eventually there will be Another Master Account with a ParentID of Null that will have childs of its own. With that in mind, am I going about this the correct way?

Thanks for the quick responses.

Answer 9

If you need recursion such as this I would personally put the recursion into a stored procedure and handle that processing on the database side in order to avoid opening multiple connections. If you are using mssql2005 look into something called Common Table Expressions (CTE), they make recursion easy. There are other ways to implement recursion with other RDBMS's.

Answer 10

Based on the sugestions I will atempt to move the query into a CTE (common table expression) when I find a good tutorial on how to do that. For now and as a quick and dirty fix, I have changed the code as follows:

Function GetChilds(ParentID, intLevel)
     'Open my Database Connection and Query the current Parent ID
     Set rsData= Server.CreateObject("ADODB.Recordset")
     sSQL = "SELECT AcctID, ParentID FROM Accounts WHERE ParentID='" & ParentID &"'"
     rsData.Open sSQL, conDB, adOpenKeyset, adLockOptimistic
     'If the Record Set is not empty continue
     If Not IsRSEmpty(rsData) Then
      Dim myAccts()
      ReDim myAccts(rsData.RecordCount)
      Dim i
      i = 0
      Do Until rsData.EOF
       Response.Write "Account ID: " & rsData("AcctID") & " ParentID: " & rsData("ParentID") & "<br />"
       'Add the Childs of the current Parent ID to an array.
       myAccts(i) = rsData("AcctID")
       i = i + 1
       rsData.MoveNext
      Loop
      'Close the SQL connection and get it ready for reopen. (I know not the best way but hey I am just learning this stuff)
      rsData.close: set rsData = nothing
      'For each Child found in the previous query, now lets get their childs.
      For i = 0 To UBound(myAccts)
       Call GetChilds(myAccts(i), intLevel + 1)
      Next
     End If
    End Function

    Call GetChilds(1,0)