2
\$\begingroup\$

I have a loop in my main sub routine which creates multiple graphs. This is run across a number of sheets, creating the same graphs for each sheet, but with different ranges and other variables.

I have a separate subroutine for each graph (two are shown below, others are similar and can be posted if necessary), however it would seem more logical to organize this better, specifically using one sub with more variables to insert a graph, as I expand and add more graphs.

I can broadly see two solutions:

Make a sub routine with a large case select statement

Create a class to do this

I'm leaning towards the latter, as it seems like a neater way to achieve what I wish. Through what I've read from CPearson and stackoverflow, I can use classes to set/let/get properties, but I'm unsure as to the best way to organize the code overall.

Do I:

Use a method to let/set all properties at once, then insert a graph by getting all of these

set each property individually in the main sub routine

Call a subroutine to specifically set the values of a class module

Or, leave the class altogether and just:

Use public variables defined in the main/ subroutine

Pass variables to the insert graph sub routine

Code for calling the insert graph sub routines is directly below, followed by the graphs themselves:

For Each b In rngQueries
 Set ToPrint = Worksheets(b.Value).Range("A" & Rows.Count).End(xlUp).Offset(3, 0)
 b.Offset(0, 3).Value = ToPrint.Address
 b.Offset(0, 4).Value = Worksheets(b.Value).Range("A1000").End(xlUp).Address
 Select Case b.Offset(0, 2).Value
 Case "Today"
 InsertBar ToPrint, b.Offset(0, 3).Value, b.Offset(0, 4).Value
 Case "TimeSeries"
 InsertLine ToPrint, b.Offset(0, 3).Value, b.Offset(0, 4).Value
 End Select
Next

Graphs:

Sub InsertLine(ToPrint As Range, PosTopLeft As String, PosBottomLeft As String)
 Dim strRange As String
 Dim rngChart As Range
 Dim myChart As Chart
 lngStartRow = Sheets(ToPrint.Worksheet.Name).Range(PosTopLeft).Row
 lngEndRow = Sheets(ToPrint.Worksheet.Name).Range(PosBottomLeft).Row
 Sheets(ToPrint.Worksheet.Name).Activate
 Sheets(ToPrint.Worksheet.Name).Range("$A$" & CStr(lngStartRow) & ":$C$" & CStr(lngEndRow)).Select
 Set myChart = ActiveSheet.Shapes.AddChart(xlLine, 500, 200).Chart
 With myChart
 .ChartArea.Format.TextFrame2.TextRange.Font.Size = 8
 .HasTitle = True
 .ChartTitle.Text = ToPrint.Worksheet.Name & " Hits & Attempts - (Last 14 Days)"
 .SeriesCollection(1).Name = Range("B" & lngStartRow - 1).Value
 .SeriesCollection(2).Name = Range("C" & lngStartRow - 1).Value
 End With
End sub
Sub InsertBar(ToPrint As Range, PosTopLeft As String, PosBottomLeft As String)
 Dim strRange As String
 Dim rngChart As Range
 Dim myChart As Chart
 lngStartRow = Sheets(ToPrint.Worksheet.Name).Range(PosTopLeft).Row
 lngEndRow = Sheets(ToPrint.Worksheet.Name).Range(PosBottomLeft).Row
 Sheets(ToPrint.Worksheet.Name).Activate
 Sheets(ToPrint.Worksheet.Name).Range("$A$" & CStr(lngStartRow) & ":$D$" & CStr(lngEndRow)).Select
 Set myChart = ActiveSheet.Shapes.AddChart(xlColumnClustered, 500, 10, , 175).Chart
 With myChart
 .ChartArea.Format.TextFrame2.TextRange.Font.Size = 8
 .HasTitle = True
 .ChartTitle.Text = ToPrint.Worksheet.Name & " Receiving Sim Stats - (Today Only)"
 .SeriesCollection(1).Name = Range("B" & lngStartRow - 1).Value
 .SeriesCollection(2).Name = Range("C" & lngStartRow - 1).Value
 .SeriesCollection(3).Name = Range("D" & lngStartRow - 1).Value
 End With
End Sub

To summarize, I am currently passing ToPrint, PosTopLeft and PosBottomLeft to the subroutine from a case select in the main sub, and defining some constants in the graphs sub routine, such as graph position, title etc. It seems more logical to organize this better. Which is the neatest, most logical way to proceed?

Finally, I should say that I am relatively new to this board, if the question is not correctly presented, or not appropriate for here, please let me know and I'll update/ remove. Thanks in advance for any help you can give.

asked Nov 29, 2016 at 14:07
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I'm not sure a class is exactly what you want - a chart is already an object with the properties/methods you want. You want to create a sub that takes the arguments you need, like

Sub CreateChart(ByVal targetRange As Range, ByVal PosTopLeft As String, byval PosBottomLeft As String, ByVal lastColumn As Long, ByVal chartType As XlChartType)
 Dim strRange As String
 Dim rngChart As Range
 Dim myChart As Chart
 lngstartrow = Sheets(ToPrint.Worksheet.Name).Range(PosTopLeft).Row
 lngendrow = Sheets(ToPrint.Worksheet.Name).Range(PosBottomLeft).Row
 Sheets(ToPrint.Worksheet.Name).Activate
 Sheets(ToPrint.Worksheet.Name).Range(.Cells(lngstartrow, 1), (.Cells(lngendrow, lastColumn))).Select
 Select Case XlChartType
 Case xlLine
 Set myChart = ActiveSheet.Shapes.AddChart(xlLine, 500, 200).Chart
 Case xlColumnClustered
 Set myChart = ActiveSheet.Shapes.AddChart(xlColumnClustered, 500, 10, , 175).Chart
 End Select
End Sub

And then you can have some variable that picks the number of series by the type of chart, then add all the series with a loop. You can also have a string that picks the title based on case.

The key is to combine as many parts of the process into a single process. Combining something a little like this -

Case xlLine
 numberofseries = 2
Case xlColumnClustered
 numberofseries = 3
For i = 1 To numberofseries
 .SeriesCollection(i).Name = .Cells(lngstartrow - 1, i + 1).Value
Next

So your case statements don't have to do very much, all they need to do is assign the proper variables to be used in the next parts.

answered Mar 22, 2018 at 21:41
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.