Search code examples
excelloopsgotovba

Error when using GoTo statement for If/Else


I'm using the following code to adjust the appearance of the chart elements only if there is data in the associated column:

mcwb.ChartObjects("Combined RTS").Activate
k = 3
Dim c1 As Range
Dim c2 As Range

Do Until wb.Worksheets("RTS Raw Data").Cells(1, k) = ""
  Set c1 = Cells(3, k)
  Set c2 = Cells(lr, k)

  With wb.Worksheets("RTS Raw Data")
    If (.Cells(3, k) <> "-999") Then
    ActiveChart.SeriesCollection.Add Source:=.Range(.Cells(3, k), .Cells(lr, k))
        GoTo Line1
    Else
        GoTo Line2
    End If
  End With

Line1:
  ActiveChart.SeriesCollection(11 + k).XValues = wb.Worksheets("RTS Raw Data").Range("B3:B" & lr)
  ActiveChart.SeriesCollection(11 + k).Name = wb.Worksheets("RTS Raw Data").Cells(2, k)
  ActiveChart.SeriesCollection(11 + k).AxisGroup = xlPrimary
Line2:
  If (wb.Worksheets("RTS Raw Data").Cells(3, k) = "-999") Then
    mcwb.Shapes("CheckBox" & 8 + k).TextFrame.Characters.Text = "Unused"
  Else
    mcwb.Shapes("CheckBox" & 8 + k).TextFrame.Characters.Text = wb.Worksheets("RTS Raw Data").Cells(2, k)
  End If

  k = k + 1
Loop

When this runs however, code within the label Line1 return improper usage errors. My goal here is to reduce the amount of instructions used while running the script and time spent in the loop. Thanks in advance.


Solution

  • The error is happening because you're using a GoTo within a With block and the block is being taken out of scope. Avoid the With block, or avoid the GoTo statements (usually frowned upon except for very specific uses).

    This can also be improved by using a Chart and Series and Worksheet object which will make the code a bit more legible :) We can also improve the nesting of the if/else condition using If/ElseIf/Else, and you should use your variables (e.g., c1, c2 instead of repetitively referring to them by their .Cells identifiers :)

    Dim cht as Chart
    Dim srs as Series
    Dim wsRTS as Worksheet
    Set cht = ActiveChart
    Set wsRTS = wb.Worksheets("RTS Raw Data")
    
    With wsRTS
        Do Until .Cells(1, k) = ""
          'If these Cells are on wsRTS, you could put the entire DO LOOP inside the WITH block.
          Set c1 = .Cells(3, k)
          Set c2 = .Cells(lr, k)
    
          If c1.Value <> "-999" Then
              'Use your c1,c2 range objects to define the series range!
              Set srs = cht.SeriesCollection.Add(Source:=.Range(c1, c2))
              srs.XValues = .Range("B3:B" & lr).Value
              srs.Name = .Cells(2, k)
              srs.AxisGroup = xlPrimary
          ElseIf c1.Value = "-999" Then
              mcwb.Shapes("CheckBox" & 8 + k).TextFrame.Characters.Text = "Unused"
          Else
              mcwb.Shapes("CheckBox" & 8 + k).TextFrame.Characters.Text = .Cells(2, k)
          End If
          k = k + 1
        Loop
    End With
    

    If you have difficulty implementing the above, start with what's below and try to implement some of the changes piece-by-piece in order to clean up your code with the above suggestions. For example, you Set c1 but never use it, even though you refer to that object (.Cells(3,k)) elsewhere in the code, getting a handle on the Series object means you can use that instead of ActiveChart.SeriesCollection(_index_) which is a clumsy construct, you're using a With wb.Worksheets("RTS Raw Data") but repeatedly referring to that object explicitly inside the With block which defeats the purpose of using a With block in the first place!, etc.:

    Do Until wb.Worksheets("RTS Raw Data").Cells(1, k) = ""
      Set c1 = Cells(3, k)
      Set c2 = Cells(lr, k)
    
      With wb.Worksheets("RTS Raw Data")
        If (.Cells(3, k) <> "-999") Then
            ActiveChart.SeriesCollection.Add Source:=.Range(.Cells(3, k), .Cells(lr, k))
            ActiveChart.SeriesCollection(11 + k).XValues = wb.Worksheets("RTS Raw Data").Range("B3:B" & lr)
            ActiveChart.SeriesCollection(11 + k).Name = wb.Worksheets("RTS Raw Data").Cells(2, k)
            ActiveChart.SeriesCollection(11 + k).AxisGroup = xlPrimary
        Else
            If (wb.Worksheets("RTS Raw Data").Cells(3, k) = "-999") Then
                mcwb.Shapes("CheckBox" & 8 + k).TextFrame.Characters.Text = "Unused"
            Else
                mcwb.Shapes("CheckBox" & 8 + k).TextFrame.Characters.Text = _ wb.Worksheets("RTS Raw Data").Cells(2, k)
            End If
        End If
      End With
      k = k + 1
    Loop