1

I'm trying to automate the process of compiling quarterly data from different files into a single Excel workbook (this needs to be done each quarter). Now I have come to a point where I wondered if I should keep all code in the main sub or if I should use a function. I really would like to hear from people who have worked with VBA for more than a few months and learn about writing efficient and readable code.

The first thing my code needs to do is getting me three date/date related variables:

  • The current year or, in case of Q4 data, the year before that
  • Depending on the quarter, a variable that looks like this: 12.31.2018 for Q4 2018; for Q1 2019 it would be 03.31.2019. It's the subfolder name where the relevant files are saved.
  • The quarter, so Q1, Q2, Q3 or Q4.

See below for my code and please improve it if something catches your eye. Specifically, I wonder how to name my variables (if you have a variable storing the year, what would you call it?). Also, the Case Else isn't necessary, right? I mean month(Date) can only be one of 12 months.

Sub DetermineDate()

Dim qVar As String
Dim yVar As Integer
Dim fullDate As String

yVar = Year(Date) 'set value here or each time in case statement?

Select Case month(Date)
    Case 1, 2, 3
        qVar = "Q4"
        yVar = Year(Date) - 1
        fullDate = "12.31." & yVar
    Case 4, 5, 6
        qVar = "Q1"
        fullDate = "03.31." & yVar
    Case 7, 8, 9
        qVar = "Q2"
        fullDate = "06.30." & yVar
    Case 10, 11, 12
        qVar = "Q3"
        fullDate = "09.30." & yVar
    Case Else 
        MsgBox "Error"
        Exit Sub

End Select
End Sub

However, my question is: is there any reason not to put this in the main sub? I had this notion that it would be good to focus on the meat of the code (copy-pasting all the data) in the main sub. So I felt it would make sense put the code to determine the date in a separate function. However, I found out returning multiple values is not straightforward and not the intention of a function. Also, I would only call this function a single time, so after some thinking I came to the conclusion it probably doesn't make sense to put this somewhere else. Basically I'm asking what is good practice when using functions and multiple subs.

NoNameNo123
  • 625
  • 1
  • 8
  • 17
  • Code reusability is a massive part of it. If you find yourself writing the same section of code that essentially does the same thing, parameterise it and throw it in a function so it can be used again. It’s programming best practice in general. There’s so much more to it but it’s a major component. – Skin Apr 27 '19 at 08:46
  • 1
    In VBA returning multiple values is simple. You use either an array, a collection/dictionary, a user defined type or an object. – freeflow Apr 27 '19 at 09:09
  • @Freeflow How would I go about deciding which one to use here? – NoNameNo123 Apr 27 '19 at 10:04
  • Simples. Why am I not using an object? – freeflow Apr 27 '19 at 12:11

1 Answers1

2

You can do something like this.

Add the below code to a new CLASS MODULE in the VBA editor and be sure to give that new class a name of clsObject ...

Public Quarter As Integer
Public Year As Integer
Public EndDate As Date

Property Get EndMonth() As Integer
    EndMonth = Month(EndDate)
End Property

Property Get QuarterText() As String
    QuarterText = "Quarter " & Quarter
End Property

Property Get DateString() As String
    DateString = Right("0" & EndMonth, 2) & "." & Day(EndDate) & "." & Year
End Property

Now, add the below code to a new MODULE in the VBA editor ...

Function DetermineDate(ByVal dtDate As Date) As clsObject
    ' Initialise the current functions return value.
    ' This is only required if the return value is an object that is yet to be assigned a value
    Set DetermineDate = New clsObject

    ' If the year is a simple derivation that is not specific to the month, do it here.
    ' You can also set it here first if it is the default for the vast majority of cases
    ' and then in the case statement, set it to something different if required.
    DetermineDate.Year = Year(dtDate)

    Select Case Month(dtDate)
        Case 1, 2, 3
            DetermineDate.Quarter = 4
            DetermineDate.EndDate = DateSerial(DetermineDate.Year, 12, 31)
            DetermineDate.Year = DetermineDate.Year - 1

        Case 4, 5, 6
            DetermineDate.Quarter = 1
            DetermineDate.EndDate = DateSerial(DetermineDate.Year, 3, 31)

        Case 7, 8, 9
            DetermineDate.Quarter = 2
            DetermineDate.EndDate = DateSerial(DetermineDate.Year, 6, 30)

        Case 10, 11, 12
            DetermineDate.Quarter = 3
            DetermineDate.EndDate = DateSerial(DetermineDate.Year, 9, 30)

        Case Else
            ' For this scenario, you don't need an else but it's here anyway.
            Exit Function

    End Select
End Function

Public Sub CallDetermineDate()
    Dim objResult As clsObject

    Set objResult = DetermineDate(Now)

    Debug.Print "End Date = " & objResult.EndDate
    Debug.Print "Year = " & objResult.Year
    Debug.Print "End Month = " & objResult.EndMonth
    Debug.Print "Quarter = " & objResult.Quarter
    Debug.Print "Quarter Text = " & objResult.QuarterText
    Debug.Print "Date String = " & objResult.DateString
End Sub

... now run the CallDetermineDate routine and you will see the output.

Whether I have your logic setup correctly or not is beside the point, it's trying to illustrate the usage of different objects within VBA and how they can hang together.

BTW - a function can return a value, sub's can not.

There are usually a multitude of ways to do things in VBA and some ways are better than others but a lot of the times, it depends on the scenario and how far you need to take the solution, i.e. edge cases, size of data sets, performance vs readability, etc.

The above could be done a few different ways as well, especially DetermineDate, all of that code could actually form a constructor (well kind of) for the class module itself rather than hanging out in the module world. Again, there are heaps of ways to do things.

Hopefully it kind of explains itself, if not, ask away but I hope that helps even if only a little bit.

Skin
  • 9,085
  • 2
  • 13
  • 29