3
\$\begingroup\$

I want to simplify some code I have written but not sure how to go about doing it. When my workbook opens it checks the current date and compares it to dates I have on a sheet. If the current date is <= date on sheet it writes the date range to a cell. My issue is I have 13 If statements and would like to simply/neaten it up.

I know there is a better way to go about if/then statements I just can't remember what it is or how to implement it.

Here is my code:

Private Sub Workbook_Open()

If ActiveWorkbook.Sheets("Period Dates").Range("C15").Value < Date Then
    ActiveWorkbook.Sheets("Period Dates").Range("C3").Value = ActiveWorkbook.Sheets("Period Dates").Range("C15") + 28
End If

If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C15").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B15").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C15").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B15").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C15").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C14").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B14").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C14").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B14").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C14").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C13").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B13").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C13").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B13").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C13").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C12").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B12").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C12").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B12").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C12").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C11").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B11").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C11").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B11").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C11").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C10").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B10").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C10").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B10").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C10").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C09").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B09").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C09").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B09").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C09").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C08").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B08").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C08").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B08").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C08").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C07").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B07").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C07").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B07").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C07").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C06").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B06").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C06").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B06").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C06").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C05").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B05").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C05").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B05").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C05").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C04").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B04").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C04").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B04").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C04").Value
End If
If Date <= ActiveWorkbook.Sheets("Period Dates").Range("C03").Value Then
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B03").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C03").Value
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = ActiveWorkbook.Sheets("Period Dates").Range("B03").Value & " - " & ActiveWorkbook.Sheets("Period Dates").Range("C03").Value
End If

End Sub
\$\endgroup\$
5
  • 2
    \$\begingroup\$ Welcome to CodeReview. Your current title applies to virtually every question on this site, so the usual practice is to title your post with what the code is doing. \$\endgroup\$
    – Teepeemm
    Nov 26, 2021 at 14:51
  • 2
    \$\begingroup\$ Welcome to the Code Review Community. Please read How do I ask a good question?. It will give you a hint on who to title the question. \$\endgroup\$
    – pacmaninbw
    Nov 26, 2021 at 15:00
  • \$\begingroup\$ @Teepeemm When a new comer to the site doesn't know that we are looking for a description of what the code does in the title, it is best to send them to the help page. \$\endgroup\$
    – pacmaninbw
    Nov 26, 2021 at 15:03
  • \$\begingroup\$ Is there any known/guaranteed relation between the values in C03…15? \$\endgroup\$
    – greybeard
    Nov 26, 2021 at 20:20
  • \$\begingroup\$ So the dates are a period of time, C03 is the start date and C15 is the end date \$\endgroup\$
    – Ryan Knox
    Nov 26, 2021 at 21:56

1 Answer 1

3
\$\begingroup\$

DRY

There is an important software principle called "Don't Repeat Yourself" (DRY). Writing software with frequent use of Copy-Paste is the antithesis of DRY. Not only does it cause repeated expressions with minor differences per copy - it results in code that is difficult to change, debug, and maintain. The purpose of the code is lost in all the typing.

Code-Behind/Error Handling

The code provided is initiated by the Open_Workbook event. When your users open this workbook, the Excel application/workbook becomes the initial User-Interface. It is usually a best practice to have as little logic as possible (other than UI logic) in these objects. The code in UI objects is generally referred to as the 'code-behind'. If it is important, in general, to have as little code as possible in the code-behind...then it is especially important when the Workbook object acts as a UI.

The more code/logic within the Workbook object (or any object for that matter), the greater the chance of errors and exceptions. If simply opening the workbook is going to 'kick-off' any kind of operation, then it would seem especially prudent to gracefully handle startup errors.

So, it would be advisable to move all the initialization code to a dedicated Standard Module. Call this module from the Workbook_Open event and protect against any errors/exceptions propagating back to the Workbook object without a handler. Further, the call to the dedicated module can take Date object as a parameter. Doing so makes it easy to test or initiate the setup code without having to close and re-open the workbook (example provided below).

The objects below implement the suggestions above.

ThisWorkbook

Option Explicit

Private Sub Workbook_Open()
    If Not SetupModule.TrySetupWorksheets(Date) Then
        MsgBox "Error encountered during startup"
    End If
End Sub

SetupModule.bas

Option Explicit

'Initiate the process for test/debugging by calling this macro from Developer -> Macros
Public Sub Test()
    If TrySetupWorksheets(Date) Then
        MsgBox "TestFailed"
    End If
End Sub

Public Function TrySetupWorksheets(ByVal currentDate As Date) As Boolean

    TrySetupWorksheets = False
    
On Error GoTo ErrorExit
    Dim periodDatesSheet As Worksheet
    Set periodDatesSheet = ActiveWorkbook.Sheets("Period Dates")
    
    SetupPeriodDates currentDate, periodDatesSheet
    
    Dim columnCValue As Variant
    
    Dim rowNumber As Long
    For rowNumber = 15 To 3 Step -1
        columnCValue = periodDatesSheet.Range("C" & CStr(rowNumber)).Value
        If currentDate <= columnCValue Then
            WriteDateExpressionToCells periodDatesSheet.Range("B" & CStr(rowNumber)).Value & " - " & columnCValue
        End If
    Next
    TrySetupWorksheets = True
    
ErrorExit:
End Function

Private Sub SetupPeriodDates(ByVal currentDate As Date, ByVal periodDatesSheet As Worksheet)
    If periodDatesSheet.Range("C15").Value < currentDate Then
        periodDatesSheet.Range("C3").Value = periodDatesSheet.Range("C15") + 28
    End If
End Sub

Private Sub WriteDateExpressionToCells(ByVal dateExpression As Variant)
    ActiveWorkbook.Sheets("North Reading").Range("B2").Value = dateExpression
    ActiveWorkbook.Sheets("Chelmsford St").Range("B2").Value = dateExpression
End Sub
  
End Sub

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