05-13-2008, 11:44 AM
I am writing a macro that groups an indented list. I am using a recursive function to discover levels and perform the grouping. Here is the function:
Private Function GroupSub(iRow As Integer, iLev As Integer) As Integer
Static i As Integer
Static flInd As Boolean
flInd = False
i = iRow
Do Until flInd = True
Select Case Cells(i, 1)
Case Is > iLev
i = GroupSub(i, Cells(i, 1))
Case Is < iLev
flInd = True
i = i + 1
Range((iRow + 1) & ":" & (i - 1)).Group
GroupSub = i
While troubleshooting, I set up a watch to track the variables i and iRow. I discovered that when the line "i = i + 1" is executed, iRow is also incremented. I can't figure out why. Any ideas?
Woot, nice bug, never seen something quite like that before! :cool:
The problem is that you have a short-circuit in your callstack's data structure. The function receives iRow "ByRef", meaning that instead of creating a new variable and setting it to the value of the argument, iRow is simply a different name for the exact same variable that was passed to the function. So, when the function recursively calls itself and passes i as the argument, iRef isn't just set to the value of i but is used as a pointer to i.
So far, so good, no problem yet. However, since i is a STATIC variable, changing i in the current scope of the function will also change i in the scope which it was called from, and since iRow is just a pointer to i... you get the point. In effect, in your entire callstack all the i's and iRow's are just different name for one single variable, any change in one will affect all the others.
Since I assume you have a reason for making i Static, the way to break out of the circle is to declare the parameter as "ByVal iRow As Integer" in the function header. Unfortunately, VB defaults to ByRef - this is generally criticized, and your case is a perfect illustration as to why it is dangerous.
If you want to read more about parameters and scope, have a look at the "Anatomy of a Function (http://www.xtremevbtalk.com/showthread.php?t=211709)" and "Standards & Practices (http://www.xtremevbtalk.com/showthread.php?t=68153)" tutorials.
Welcome to the forum! :)
05-13-2008, 01:51 PM
Thanks for the reply. That fixed it. I actually didn't have a reason to keep i as a static. I typically use static variables for efficiency and to minimize scope. I thought it might be a reference problem, but I wasn't sure where the reference originated. I just used 'dim i as integer' and it cleaned it right up. Thank you again.
I typically use static variables for efficiency and to minimize scope.
That's not a good habit. Even in cases in which you don't deliberately put in recursion, there's always the possibility of a function being run several times at once (A calls B calls C calls A), and static variables will almost certainly get you into trouble in a case like that. And it's the sort of trouble that can be very hard to track down.
Anyway, glad your macro is fixed. :)