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.

\$\endgroup\$
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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.