charwayne
charwayne

Reputation: 103

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.

Upvotes: 0

Views: 1399

Answers (1)

David Zemens
David Zemens

Reputation: 53623

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

Upvotes: 2

Related Questions