Solved: Private Sub Help

[rossmiddleto]

Hi everyone, I have a private sub that updates two line charts when a drop down selection box selection is changed (kindly written by XLD from this forum for one chart with my own modifications) . The first chart is working/updating perfectly but the second chart keeps on throwing up an error for the follwoing line

.SeriesCollection(1).Name = "HH Price2"

"Run-time error 438
object doesnt support this property or method"


The position of this line is indicated in the code below.


Any ideas??




'const
Private Sub Worksheet_Change(ByVal target As Range)
Const WS_RANGE As String = "A2"

Dim rng As Range
Dim rng2 As Range

'On Error GoTo ws_exit

Application.EnableEvents = False

If Not Intersect(target, Me.Range("A2")) Is Nothing Then '

With target

Select Case .Value

Case "Oil":

Set rng = Me.Range(Me.Range("E4"), Me.Range("E4").End(xlToRight).Offset(0, -2))
Me.ChartObjects("Chart 6").Chart.SetSourceData Source:=rng

With Sheets("sheet3")
Set rng2 = Me.Range(Me.Range("B4"), Me.Range("B4").End(xlToRight))
End With

Me.ChartObjects("Chart 3").Chart.SetSourceData Source:=Sheets("sheet3").Range("rng2")


With ChartObjects("Chart 6").Chart
.HasTitle = True
.ChartTitle.Text = target.Value
.SeriesCollection(1).Name = "HH Price"
End With

With ChartObjects("Chart 3").Chart
.HasTitle = True
.ChartTitle.Text = target.Value
.ActiveChart.SeriesCollection(1).Name = "Daily Close 2"
End With


Case "Gas"

Set rng = Me.Range(Me.Range("e5"), Me.Range("e5").End(xlToRight).Offset(0, -2))
Me.ChartObjects("Chart 6").Chart.SetSourceData Source:=rng

With ChartObjects("Chart 6").Chart
.HasTitle = True
.ChartTitle.Text = target.Value
.SeriesCollection(1).Name = "HH Price2" <------ERROR LINE
End With




End Select
End With
End If

'ws_exit:
Application.EnableEvents = True
End Sub

[p45cal]

It would be good if you could post the relevant (parts of) the workbook, because there are few questions:

1. this snippet of your code:
With Sheets("sheet3")
Set rng2 = Me.Range(Me.Range("B4"), Me.Range("B4").End(xlToRight))
End Withuses With but there is nothing prefixed with just a dot, so the With bit is redundant.)
And this raises a second Q.:

2. Is this code in sheet3's code module or in another?

3. At another place you have: With ChartObjects("Chart 3").Chart
.HasTitle = True
.ChartTitle.Text = target.Value
.ActiveChart.SeriesCollection(1).Name = "Daily Close 2"
End With
where I wouldn't have expected to see ActiveChart in there at all, rather I feel it should be deleted because
ChartObjects("Chart 3").Chart.ActiveChart.SeriesCollection(1).Name = "Daily Close 2" may lend itself to an error if that chart doesn't happen to be the active one.

4.You set a range object called rng2 and later you have in a line:
Source:=Sheets("sheet3").Range("rng2")
Is this supposed to be the same range? I bet you're using Excel 2007 or later. 2 points:
a) Range("rng2") isn't rng2; the line should probably read:
Me.ChartObjects("Chart 3").Chart.SetSourceData Source:=rng2 b) In Excel 2007 up, Range("rng2") is a single cell in the second row of column RNG!

Regarding point (b), if it's an empty cell, you won't have a seriescollection(1) for that chart. So it might be difficult to assign it a name.

So if you could attach a file demonstrating the error containing any data that you want used in the charts it would be very helpful. (At first glance there seems to be nothing wrong with the line that you say causes an error.)

[rossmiddleto]

Hi p45cal,

First let me say that I really appreciate the detailed nature of your reply. I only started learning VBA two weeks ago and it is replies like yours that make the learning process much easier for beginners like me.

I have tried making the changes that you have suggested and have some answers for you:


1) Inserted dots before code for use with the 'with' statement - I don’t know the difference between using a 'with' statement instead of referring to the sheet in the line of code such as sheets("sheet3").range... so if you could give me some guidance around that that would be awesome!

2) This code is in sheet1's code module - Is this the reason why I can’t refer to sheet 3 for my second charts source data but i have no problem referring to source data for the first chart as the data is in sheet1?

3) Removed Active chart reference as the 'with' statement should cover this right?

4) Changed rng2 to 'secondrange' to get around any wrong references.







'const
Private Sub Worksheet_Change(ByVal target As Range)
Const WS_RANGE As String = "A2"

Dim rng As Range
Dim secondrange As Range

'On Error GoTo ws_exit

Application.EnableEvents = False

If Not Intersect(target, Me.Range("A2")) Is Nothing Then '

With target

Select Case .Value

Case "Oil":
'set source for first chart
Set rng = Me.Range(Me.Range("E4"), Me.Range("E4").End(xlToRight).Offset(0, -2))
Me.ChartObjects("Chart 6").Chart.SetSourceData Source:=rng

'set source for second chart
With Sheets("sheet3")
.Set secondrange = Me.Range(Me.Range("B4"), Me.Range("B4").End(xlToRight))
End With

Me.ChartObjects("Chart 3").Chart.SetSourceData Source:=Range.secondrange

'set series name for first chart
With ChartObjects("Chart 6").Chart
.HasTitle = True
.ChartTitle.Text = target.Value
.SeriesCollection(1).Name = "HH Price"
End With

'set series name for second chart
With ChartObjects("Chart 3").Chart
.HasTitle = True
.ChartTitle.Text = target.Value
.SeriesCollection(1).Name = "Daily Close 2"
End With


Case "Gas"

Set rng = Me.Range(Me.Range("e5"), Me.Range("e5").End(xlToRight).Offset(0, -2))
Me.ChartObjects("Chart 6").Chart.SetSourceData Source:=rng

With ChartObjects("Chart 6").Chart
.HasTitle = True
.ChartTitle.Text = target.Value
.SeriesCollection(1).Name = "HH Price"
End With




End Select
End With
End If

'ws_exit:
Application.EnableEvents = True
End Sub




I have attached my workbook in case it is of use.

Thanks again,

Kind Regards

Rossa

[p45cal]

See attachment.

Just a quick one on the With and the dots:
When you use the likes of
With Something.Blah.Thingy.Blish
.text = "Hello"
.font = Bold
.Italic = True
.Colour = vbGreen
xxx = "All Rubbish"
Set yyy= Range("S3")
End With
it's the same as coding:
Something.Blah.Thingy.Blish.text = "Hello"
Something.Blah.Thingy.Blish.font = Bold
Something.Blah.Thingy.Blish.Italic = True
Something.Blah.Thingy.Blish.Colour = vbGreen
xxx = "All Rubbish"
Set yyy= Range("S3") (All invented properties etc. of course)
I left the Set and xxx lines within the With..End With section just to show that those lines don't need a dot in from of them, but those two lines don't really need to be within the With..End section at all.

Now onto your charts:
There's a lot of repeating code, for example:
Set rng = Me.Range(Me.Range("E4"), Me.Range("E4").End(xlToRight).Offset(0, -2))
and:
Set rng = Me.Range(Me.Range("E5"), Me.Range("E5").End(xlToRight).Offset(0, -2))
where the only difference between oil and gas is the number 4 changes to 5.

Likewise:
With Sheets("sheet3")
.Set secondrange = Me.Range(Me.Range("B4"), Me.Range("B4").End(xlToRight))
End With
andWith Sheets("sheet3")
.Set secondrange = Me.Range(Me.Range("B5"), Me.Range("B5").End(xlToRight))
End With

Also you have (or intend to have, as I imagine you want further to develop the Gas part of the Select Case section to include updating the second chart) code similar to the following in each of the Select Case statements:'set series name for first chart
With ChartObjects("Chart 6").Chart
.HasTitle = True
.ChartTitle.Text = target.Value
.SeriesCollection(1).Name = "HH Price"
End With

'set series name for second chart
With ChartObjects("Chart 3").Chart
.HasTitle = True
.ChartTitle.Text = target.Value
.SeriesCollection(1).Name = "Daily Close 2"
End With
where the only difference is the collection(1).name string.
[Note: Also, even the two sections within that bit of code are similar, the only difference being which chart is being acted on - but I'll come to this later]

At the moment you're only selecting between two commodities, but what if there were a bunch of 10 or even 20 commodities to select from? That'd be a lot of copying and pasting and tweaking of code, as well as making it lengthy. What I'm trying to say is: in general, try to keep code short by looking for the repeating stuff and see if you can't have that code appear only once, and use variables for the bits that change. It'll make adjusting you code in the future easier too.

So, where the difference above is just a 4 or a 5, put that value into a variable - say RowNo. Assign it 4 if it's oil and 5 if it's gas.
Similarly, in the other repeating codes, the difference is only the collection names, so let's put those into two variables (one for each chart) CollectionName1 and CollectionName2 and have that bit of code only once.

In fact RowNo, CollectionName1 and CollectionName2 are the ONLY differences between oil and gas, so lets assign values to those variables and nothing else in the select case statement:
Select Case .Value
Case "Oil"
RowNo = 4
CollectionName1 = "HH Price"
CollectionName2 = "Daily Close Oil"
Case "Gas"
RowNo = 5
CollectionName1 = "HH Price2"
CollectionName2 = "Daily Close Gas"
End Select(I'll leave you to tweak the actual strings you want to see.)
You can see where this is going if you have more commodities - just more Case statements - 4 more lines per commodity. Full stop.

Private Sub Worksheet_Change(ByVal Target As Range)
If Not Intersect(Target, Me.Range("A2")) Is Nothing Then '
Dim rng As Range
Dim secondrange As Range
Dim RowNo As Long, CollectionName1 As String, CollectionName2 As String
'On Error GoTo ws_exit
Application.EnableEvents = False
With Target
Select Case .Value
Case "Oil"
RowNo = 4
CollectionName1 = "HH Price"
CollectionName2 = "Daily Close Oil"
Case "Gas"
RowNo = 5
CollectionName1 = "HH Price2"
CollectionName2 = "Daily Close Gas"
End Select

Set rng = Me.Range(Me.Range("e" & RowNo), Me.Range("e" & RowNo).End(xlToRight).Offset(0, -2))
With Sheets("sheet3")
Set secondrange = .Range(.Range("B" & RowNo), .Range("B" & RowNo).End(xlToRight))
End With
Me.ChartObjects("Chart 6").Chart.SetSourceData Source:=rng
With ChartObjects("Chart 6").Chart
.HasTitle = True
.ChartTitle.Text = Target.Value
.SeriesCollection(1).Name = CollectionName1
End With

Me.ChartObjects("Chart 3").Chart.SetSourceData Source:=secondrange
With ChartObjects("Chart 3").Chart
.HasTitle = True
.ChartTitle.Text = Target.Value
.SeriesCollection(1).Name = CollectionName2
End With
End With
End If

'ws_exit:
Application.EnableEvents = True
End Sub
Now the following is academic and I don't suggest you actually use it, but I said that I'd come back to that similar code where you process the two charts: Me.ChartObjects("Chart 6").Chart.SetSourceData Source:=rng
With ChartObjects("Chart 6").Chart
.HasTitle = True
.ChartTitle.Text = Target.Value
.SeriesCollection(1).Name = CollectionName1
End With

Me.ChartObjects("Chart 3").Chart.SetSourceData Source:=secondrange
With ChartObjects("Chart 3").Chart
.HasTitle = True
.ChartTitle.Text = Target.Value
.SeriesCollection(1).Name = CollectionName2
End With
Here, the only differences are "Chart 3" versus "Chart 6" and CollectionName1 versus CollectionName2. Perhaps we could shorten this too in a similar way? With just two charts, it's probably not worth it, but if you had more charts you could put the charts into an array, do the same with with the Collection Names and Source Ranges and use a loop to iterate through them. You'd end up with something like:Private Sub Worksheet_Change(ByVal Target As Range)
If Not Intersect(Target, Me.Range("A2")) Is Nothing Then '
With Target
Dim rng As Range
Dim secondrange As Range
Dim RowNo As Long, ChartsArray, SourceRangesArray, CollectionNames, cht, idx
ChartsArray = Array(Me.ChartObjects("Chart 6").Chart, Me.ChartObjects("Chart 3").Chart)
'On Error GoTo ws_exit
Application.EnableEvents = False
Select Case .Value
Case "Oil"
RowNo = 4
CollectionNames = Array("HH Price", "Daily Close Oil")
Case "Gas"
RowNo = 5
CollectionNames = Array("HH Price2", "Daily Close Gas")
End Select
Set rng = Me.Range(Me.Range("e" & RowNo), Me.Range("e" & RowNo).End(xlToRight).Offset(0, -2))
With Sheets("sheet3")
Set secondrange = .Range(.Range("B" & RowNo), .Range("B" & RowNo).End(xlToRight))
End With
SourceRangesArray = Array(rng, secondrange)
idx = 0
For Each cht In ChartsArray
With cht
.SetSourceData Source:=SourceRangesArray(idx)
.HasTitle = True
.ChartTitle.Text = Target.Value
.SeriesCollection(1).Name = CollectionNames(idx)
End With
idx = idx + 1
Next cht
End With
End If

'ws_exit:
Application.EnableEvents = True
End Sub Finally (gasp!), there's a fair bit of hard-coded stuff there - the strings used in assigning collection names - these could be fetched from the sheet instead - say columns (perhaps hidden) on the sheet, on the same row as each commodity (and not necessarily on the same sheet!), so you you could use RowNo there too to establish what their values should be. This would shorten the code even further and make it easier to make adjustment and/or add commodities and charts. Enough.

[rossmiddleto]

Genius!

Thanks for the lesson there on the 'with' statements - that should speed up my code writing no end now that i understand what they are!

I am taking your advice and trawling back through my old code to see if I can cut down on the amount of repeated code. It is going to take me a while to dissect the your new code and find out how to combine all the different elements that you used to create the final peice, but I have no doubt that as soon as I have figured it out I will be able to save loads of time in future projects!

Thank you ever so much!

[rossmiddleto]

Appologies for the pestering but I have been working for the past hour to try and add anothe line to my first chart that shows the maximum of the commodities price (the current chart just shows the current price).

I am hoping that if you could let me know how to do this then I will be able to carry on and add the same thing to the other graph as a learning exercise.

Thank you!

[p45cal]

Where would it get the max from?
The file you attached only had a single price. If you have a file with more prices (and that's where you want the max from) could you attach please.

[rossmiddleto]

Hi P45cal,

In the attached file the max price line for the first chart is coming from sheet 1.

If the chart is set to display the case "Oil" then there needs to be a series on the same chart (name the series 'Max') that displays the maximum price from the range named 'rng' in your private sub code.

I was hoping that we could have this line displayed as a continual line and not just one point on the chart. So for example, if there were 30 data points on the chart then the 'max' line would strech out across all 30 points and wouldnt just be a single point on the graph.

Kind Regards

Ross

[p45cal]

see attached

[rossmiddleto]

Massive thanky you. That is exactly what I was after.

It is going to take me a while to emulate what you have done in my other spreadsheets but like you said before, it should save me lots of time and repeated code in the future.

Thanks again

Ross