0

I honestly do not know why the VBA compiler is nagging me because of that GoTo Jump, respectively Jump:.

    counter2 = 0
    If (counter1 > 1) Then
        For i = 0 To (MaxLastCell - 4)
            If (IncompleteRows(i) = 1) Then
                If ((counter2 > 1) And (counter2 < counter1)) Then
                    x = x + ", " + CLng(i)
                    counter2 = counter2 + 1
                    GoTo Jump
                End If
                If ((counter2 > 1) And (counter2 = counter1)) Then
                    x = x + " and " + CLng(i)
                    GoTo Outside
                If (counter2 = 0) Then
                    x = CLng(i)
                    counter2 = 1
                End If
            End If
Jump:
        Next i

Every time I am trying to run my code, this code snippet appears to be a problem. The compiler marks Next at the very bottom and tells me that there is a "Next without For".

But shouldn’t this kind of coding work? I just saw it here. A strange thing, however, is that the compiler there did not seem to coerce B H to move his jumping point NextIteration: to the very left but allowed it to stay at the second indent level and, thus, within the for-loop, as it seems. (Does that even matter?)

Community
  • 1
  • 1
Qralnaq
  • 73
  • 1
  • 7
  • 2
    Change your IF structure to an ElseIF like the answer below. You can then remove the goto Jump line and replace the goto Outside with `Exit For`. – Scott Craner Nov 08 '16 at 17:19

2 Answers2

2

try this (revision marked in comments):

    counter2 = 0
    If (counter1 > 1) Then
        For i = 0 To (MaxLastCell - 4)
            If (IncompleteRows(i) = 1) Then
                If ((counter2 > 1) And (counter2 < counter1)) Then
                    x = x + ", " + CLng(i)
                    counter2 = counter2 + 1
                    GoTo Jump
                End If
                If ((counter2 > 1) And (counter2 = counter1)) Then
                    x = x + " and " + CLng(i)
                    GoTo Outside
                ElseIf (counter2 = 0) Then '<--*** changed from simple 'If'
                    x = CLng(i)
                    counter2 = 1
                End If
            End If
Jump:
        Next i
    End If '<--*** added

but you should avoid GoTos

user3598756
  • 28,893
  • 4
  • 18
  • 28
2

You've got some nice spaghetti code there. GoTo is but a poor alternative to proper control flow.

Neal Stephenson thinks it's cute to name his labels 'dengo'

One GoTo to "skip to next iteration" is one thing. Another to GoTo Outside (wherever that is) is something else.

VBA (the language spec) doesn't care on which column a line label starts; for all we know the answer you linked to was typed in the answer box, not in the VBE. When the VBE (the IDE/editor) sees a line label, it moves it to column 1 automatically, just like it automatically inserts spaces between operators and operands, and just like it automatically adjusts the casing of keywords and identifiers as you type them. So no, it doesn't matter at all.

VBA syntax requires blocks to be closed: just like a Sub DoSomething() procedure must end with End Sub and a With block must end with End With, a For block must end with Next. Proper indentation and small procedure bodies usually help getting this right.

A lot of other languages (C#, Java, C++, etc.) have similar constraints about what makes a valid code block (mismatched { and } braces are a compiler error in every language that uses them AFAIK), so this isn't VBA being picky or complaining for no reason.

That said it's hard to tell whether and where your code is malformed, because you're not including the entire procedure scope so we have to assume there's nothing else that follows your snippet - and the snippet you've posted is missing an End If as user3598756 has noted:

If (counter1 > 1) Then
    '...code...
End If

So, how to go about restructuring this?

  • Assuming the Outside line label is located just before End Sub (or is it End Function?), then you can replace it with Exit Sub (or Exit Function) and call it a day.
    • If there's more code that needs to run after the loop but before the end of the procedure scope, Exit For will get you out of the loop while keeping you inside the procedure - next line to run will be the first executable statement that follows the Next token.
  • Now take the condition that makes the loop skip an iteration, and rephrase the loop body accordingly; use ElseIf to avoid evaluating conditions you don't need to, and remove all these extraneous and confusing parentheses:

    If IncompleteRows(i) = 1 And counter2 > 1 And counter2 < counter1 Then
        x = x + ", " + CLng(i)
        counter2 = counter2 + 1
    ElseIf counter2 > 1 And counter2 = counter1 Then
        x = x + " and " + CLng(i)
        Exit For ' assuming...
    ElseIf counter2 = 0 Then
        x = CLng(i)
        counter2 = 1
    End If
    

    And that would be the entire body of the loop. Of course it could still be improved; counter2 > 1 is repeated twice, so there's room for some further restructuring. But already, all GoTo's are gone.

Community
  • 1
  • 1
Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235