Reputation: 103
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
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