Krdytkyu

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.

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.