Getting "Out of Stack Space error", not sure why/where I have recursion in my code

Question

I have an Excel/Vba program which "inputs" a "point" via command button on a grid in excel, all the rows in the column that the "point" is inputted in are then used to score the "suitability" of that point's place in the grid (for reasons which I will not get into now).

I am trying to write another command button code which say:

If I input a "point" & (due to other criteria etc), the [row 79] of that column = "p", copy the value in [row 75] of that column. (Let's call this "Aboverow"

Then search backwards (to column C) along [row 79] for all cells/columns with "r" present in them.

For every column with "r" present in it, paste "Aboverow's" value into [row 80] of that column.

I have written the code for this but I keep getting "out of stack space error", I have tried to debug it, but am not sure where I am going wrong. The code is below. Any help will be very much appreciated.

Private Sub commandButton12_Click()

    For Checkcol = 3 To 801

        Dim Isnote As Variant

        GoSub Isnote

Isnote:
        If Cells(78, Checkcol) <> "o" Then
            GoSub Isap
        Else
        End If

        Dim Isap As Variant        
Isap:
        If Cells(80, Checkcol) = "p" Or Cells(80, Checkcol) = "pf" Or Cells(80, Checkcol) = "ps" Then
            GoSub Myprocess
        ElseIf Cells(80, Checkcol) = "r" Or Cells(80, Checkcol) = "rf" Or Cells(80, Checkcol) = "rs" Then
        End If

        Dim Myprocess As Variant

        Dim Reg As Variant    
        Reg = "r"        

        Dim Rainj As Variant

        If ActiveCell.Column = Range("J79") Then
            Rainj = Range("J79") & ("C79")
        End If

        Dim Aboverow As Variant

Aboverow:
        If ActiveCell.Column = Range("J79") Then
            Range("J75").Select
            Selection.Copy
        End If

        Dim Belowrow As Variant    
Belowrow:
        If Range("I79") = Reg Then
            Range("I80").Select
        End If

Myprocess:
        GoSub Aboverow
        For Each Reg In Rainj
            GoSub Belowrow
            Selection.Paste
        Next Reg
    Next

End Sub

Answer 1

Just thought I would post the working answer for my question, having fixed the code given to me:

Sub Macro1()

Dim targetSheet As Worksheet
Set targetSheet = ActiveSheet

For I = ActiveCell.Column - 1 To 1 Step -1
If Cells(78, I).Value2 = "r" Then
Cells(80, I).Value = Cells(75, ActiveCell.Column).Value
ElseIf Cells(78, I).Value2 = "p" Then
Cells(80, I).Value = Cells(75, ActiveCell.Column).Value
Exit For
End If
Next I



End Sub

Thanks again for the replies.

Answer 2

In addition to what was already said...

• Don't declare line labels as local variables.
• Declare actual variables with actual data types. Using Variant when you mean Long can cost you some performance.
• Avoid working for nothing: if there's no code in an Else block, then there's no reason to have an Else If condition at all.
• Avoid working off the current Selection and ActiveCell. Use Range object references and work against that.
• Avoid implementing logic in click handlers - often you'll need more click handlers and you'll end up duplicating logic everywhere: that means you can have one bug, and twelve places to fix it.

So execution starts and assigns an undeclared Checkcol variable in a For loop that runs from 3 to 801.

For Checkcol = 3 To 801

That should probably be reworded to eliminate the hard-coded magic numbers:

Dim targetSheet As Worksheet
Set targetSheet = ActiveSheet 'ideally something more robust than that

Dim firstColumn As Long
firstColumn = FindFirstColumn(targetSheet) 'function that returns the first column with a heading in a specified worksheet

Dim lastColumn As Long
lastColumn = FindLastColumn(targetSheet) 'function that returns the last column with a heading in a specified worksheet

Dim currentColumn As Long
For currentColumn = firstColumn To LastColumn

Next instruction declares an Isnote variable, but that variable is really just a line label and shouldn't be declared at all.

Next we have a GoSub jump, that's redundant because it jumps to the line that would be executing next anyway - except here's the thing with GoSub: the VBA interpreter uses a mechanism called the call stack to track where it's at in the program. Because there is no Return statement, after that GoSub jump this is what the call stack might look like:

at label:Isnote
at sub:commandButton12_Click

Now, say Cells(78, Checkcol) <> "o". The call stack becomes this:

at label:Myprocess
at label:Isnote
at sub:commandButton12_Click

Now the MyProcess label starts by jumping to Aboverow, so the call stack grows again:

at label:Aboverow
at label:Myprocess
at label:Isnote
at sub:commandButton12_Click

AboveRow completes, but doesn't Return to the caller, so the last "frame" on the call stack is never "popped". Execution continues into Belowrow (without pushing the label to the stack) and then back into Myprocess (again without pushing it to the stack), and jumps back to Aboverow because that's what Myprocess does:

at label:Aboverow
at label:Aboverow
at label:Myprocess
at label:Isnote
at sub:commandButton12_Click

At that point execution is stuck: the stack will eventually fill up completely with at label:Aboverow stack frames, until it overflows its capacity and the runtime raises the run-time error you're seeing - "Out of stack space".

Let's rewind to here:

at label:Isnote
at sub:commandButton12_Click

If Cells(78, Checkcol) does contain an o, then we don't jump and everything is great, no?

Execution resumes into Isap (without pushing it to the call stack), and then conditionally jumps to Myprocess which we know is going to get us stuck. The ElseIf condition is otherwise evaluated... for nothing, because nothing happens anyway.

We assign Reg to a string value of "r", then Rainj to an interesting concatenated value of whatever string is found in Range("J79"), with the string literal value "C79", so if J79 contains Hello then that makes Rainj contain HelloC79 - I very much doubt that this is the intended behavior.

Anyway we eventually reach MyProcess, where we get stuck in that loop again.

This code is therefore unreacheable, there's no way it can ever get executed:

For Each Reg In Rainj
    GoSub Belowrow
    Selection.Paste
Next Reg

...which is a good thing, because it's trying to iterate what we now know is a string that ends with "C79". Even if Range("J79") contained something clever, e.g. "Sheet12!" so Rainj would be "Sheet12!C79", it would still be a string literal at that point, and that loop is trying to iterate it.

Looks like the intent was something like:

For Each Reg In Range(Rainj)

But what's Reg anyway? Reg contains the string literal value "r", and we're not doing anything with it... except in the BelowRow label, where we compare it against Range("I79"):

If Range("I79") = Reg Then

So when Range("I79").Value is "r" then we .Select cell I80... right? Except, that For Each loop is using Reg as an iterator, which in theory only has a meaningful value inside the body of the loop - and this is where my brain explodes and gives up trying to figure out what this is supposed to be doing: it looks like it wants to .Paste something in I80 when I79 has some hard-to-figure-out-without-a-debugger value... but then, the only thing that ever gets copied is J75 - so everything is constant here, except the value of J79.

Seems a very, very convoluted way to do:

ActiveSheet.Range("J75").Copy ActiveSheet.Range("I80")

...800 times over (well, not really, since execution can never reach the Next statement before overflowing the call stack).

Answer 3

As mentioned in the comments, this is why you should always avoid GoTo and GoSub like the plague. The immediate cause of your stack overflow is that you are calling GoSub and don't have any Return statements. That means in this section of code (the first infinite recursion I noticed, but probably not the only one)...

Aboverow:
        If ActiveCell.Column = Range("J79") Then
            Range("J75").Select
            Selection.Copy
        End If

        Dim Belowrow As Variant

Belowrow:
        If Range("I79") = Reg Then
            Range("I80").Select
        End If

Myprocess:
        GoSub Aboverow
        For Each Reg In Rainj
            GoSub Belowrow
            Selection.Paste
        Next Reg

...your exceution jumps from the line with GoSub Belowrow, then it falls right back into the code below the Myprocess: label, which jumps above the Belowrow: label, which falls back into the code below the Belowrow: label, which...you get the idea.

---

While you could experiment with using Return to exit your "Subs", that style of coding died sometime in the 1970s for this exact reason. If you need a subroutine, you should make a sub-routine. Note that this is just an example - the flow control is convoluted enough that I have no idea what the code is supposed to do:

Sub AboveRow()
    If ActiveCell.Column = Range("J79") Then
        Range("J75").Select
        Selection.Copy
    End If
End Sub

Sub BelowRow()
    If Range("I79") = Reg Then
        Range("I80").Select
    End If
End Sub

Sub Myprocess()
    AboveRow
    For Each Reg In Rainj
        BelowRow
        Selection.Paste
    Next Reg
End Sub

Then the code quoted above would become just:

    AboveRow
    BelowRow
    Myprocess

Note that there are many other issues around relying on Select and tracking the active cell, but that should at least get you closer to a point that you can start doing some more meaningful debugging on this.