HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Re-worked Workbook_Open, Creating DateStrings

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
creatingworkbook_opendatestringsworked

Problem

So, I had a go at re-writing my Workbook_Open event based on The advice from my previous question. Thoughts?

Private Sub Workbook_Open()

    Dim strTableTitle       As String
    Dim StrTodaysDate       As String
    Dim StrThisDay          As String
    Dim StrThisMonth        As String
    Dim StrThisYear         As String

    Dim strSaveFileFolder   As String
    Dim StrSaveFilename     As String
    Dim StrSaveDate         As String

    Dim SheetDashboard As Worksheet
    Set SheetDashboard = Sheets("Market Dashboard")

        SheetDashboard.Activate
        SheetDashboard.Cells(1, 1).Value = strTableTitle

        StrThisDay = day(Date)
        StrThisMonth = MonthName(month(Date))
        StrThisYear = year(Date)

        StrTodaysDate = StrDay & " " & StrMonth & " " & StrYear
        strTableTitle = "Weekly Market Recap - " & StrDateString

        strSaveFileFolder = "S:\Investments\Regular Reports\Market Insight Reports\Weekly Dashboard\"
        StrDateNumber = StrDay & "." & month(Date) & "." & StrYear
        StrSaveFilename = "Client Facing Dashboard - " & StrDateNumber & ".xlsm"

    Dim lngDay As Long, lngMonth As Long, lngYear As Long
    Dim str3MonthsAgo As String, str6MonthsAgo As String, str9MonthsAgo As String, str12MonthsAgo As String
    Dim strStartDate As String

    strStartDate = "01/" & month(Date) & "/" & year(Date)

        str3MonthsAgo = DateAdd("m", -3, strStartDate)

        str6MonthsAgo = DateAdd("m", -6, strStartDate)

        str9MonthsAgo = DateAdd("m", -9, strStartDate)

        str12MonthsAgo = DateAdd("yyyy", -1, strStartDate)

        '/ Key Rates Headers
        Cells(20, 12) = str3MonthsAgo
        Cells(20, 13) = str6MonthsAgo
        Cells(20, 14) = str9MonthsAgo
        Cells(20, 15) = str12MonthsAgo

        '/ Currencies Headers
        Cells(20, 20) = str3MonthsAgo
        Cells(20, 21) = str6MonthsAgo

        ActiveWorkbook.SaveAs filename:=strSaveFileFolder & StrSaveFilename

End Sub

Solution

First thought: It feels really rewarding to see a new User who takes on advice that's offered and proactively uses it to improve.

Second thought: MUCH, MUCH better. Now if you (or somebody else) ever has to come back to this code, it's going to be crystal clear what everything does and how it all works.

One more improvement:

Stop this right now.

Dim strTableTitle       As String
Dim StrTodaysDate       As String
Dim StrThisDay          As String
Dim StrThisMonth        As String
Dim StrThisYear         As String

Dim strSaveFileFolder   As String
Dim StrSaveFilename     As String
Dim StrSaveDate         As String


I get it, it looks really pretty. I was exactly the same when I started. Here's the thing though good code isn't pretty. Clear, concise, consistent, tidy, but not necessarily pretty. Pretty takes time. Time that could be spent actually improving your code.

Plus, if you ever change any of those variable names, it's going to be a real pain getting it all to line up again, especially if you write one that's longer than the current ones.

Code Snippets

Dim strTableTitle       As String
Dim StrTodaysDate       As String
Dim StrThisDay          As String
Dim StrThisMonth        As String
Dim StrThisYear         As String

Dim strSaveFileFolder   As String
Dim StrSaveFilename     As String
Dim StrSaveDate         As String

Context

StackExchange Code Review Q#102382, answer score: 5

Revisions (0)

No revisions yet.