Convert ISO 8601 week to American-formatted date

Question

Requirements:

Implement a function that, given a standard date and an indicator whether or not the output should be in american standard (weeks start on sundays, the first week of the year is the week containing the first day of the year) or in ISO8601 standard (weeks start on mondays, the first week of the year is the week containing the first thursday of the year) outputs the calendar week and year of the input date in the following format as a string.

[calendar week] / [year]

Further requirements:

• camelCase is to be used for function names
• variables are to be underscore-seperated / lower_snake_case
• option explicit is mandatory

This is my implementation:

Option Explicit
Public Function convertDateToFormattedCalendarWeek(input_date As Date, mode As Integer) As String

Dim week As String
Dim year As String
If mode = 0 Then 'mode 0 = standard american timekeeping
    week = CStr(DatePart("ww", input_date, vbSunday, vbFirstJan1))
    year = CStr(DatePart("yyyy", input_date, vbSunday, vbFirstJan1))
ElseIf mode = 1 Then 'mode 1 = ISO8601 business timekeeping
   'using the thursday in the given week instead for calculation to avoid the datepart leap year bug
    week = CStr(DatePart("ww", input_date - Weekday(input_date, vbMonday) + 4, vbMonday, vbFirstFourDays))
    year = CStr(DatePart("yyyy", input_date - Weekday(input_date, vbMonday) + 4, vbMonday, vbFirstFourDays))
Else
    convertDateToFormattedCalendarWeek = "error"
    Debug.Print ("function convertDateToFormattedCalendarWeek called with invalid parameter mode " & mode)
    Exit Function
End If

convertDateToFormattedCalendarWeek = week & " / " & year

End Function

Runs fine from what I've tested (a lot of dates)

Answer 1

convertDateToFormattedCalendarWeek = "error"
Debug.Print ("function convertDateToFormattedCalendarWeek called with invalid parameter mode " & mode)

Bad idea. A bad function call involving invalid parameters is a programming bug, and should be a serious run-time error that takes down the whole thing. By returning a String you're forcing the caller to systematically verify that it's not getting the "error" magic string literal.

The function has one job. If it can't do it, it should blow up: there's a run-time error specifically for this:

Err.Raise 5, "ConvertDateToFormattedCalendarWeek", _
             "Invalid 'mode' parameter value: " & mode

Run-time error 5 is essentially your ArgumentException. If I consume your function and pass an invalid parameter, I'm expecting that error to be raised.

But what's that mode anyway?

Public Function convertDateToFormattedCalendarWeek(input_date As Date, mode As Integer) As String

From the signature, I've no idea. [Seriously] consider using an enum instead:

Public Enum WeekFormattingMode
    WFM_AmericanStandard
    WFM_ISO8601
End Enum

And now your signature can be (note, member names should be PascalCase):

Public Function ConvertDateToFormattedCalendarWeek(input_date As Date, mode As WeekFormattingMode) As String

Now the caller gets auto-completion when calling the function, and has to work very hard to supply an invalid parameter value.

This also eliminates magic constants in your implementation, and replaces the otherwise needed comments, which makes the code more self-descriptive:

Select Case mode
    Case WFM_AmericanStandard
        'handle American Standard formatting

    Case WFM_ISO8601
        'handle ISO-8601 formatting

    Case Else
        Err.Raise 5, "ConvertDateToFormattedCalendarWeek", _
                     "An invalid 'mode' parameter was supplied"
End Select

The parameters are implicitly passed ByRef, but they're not assigned and the caller doesn't need to use them - they're really read-only inputs that the function needs in order to work with; such parameters should be passed ByVal.

I don't agree with all your naming conventions, but as long as it's consistent, it should be good. I like that you're not using the very commonly abused Systems Hungarian notation. Option Explicit shouldn't be part of a convention though - it should be mandatory for any VBA code that isn't toy code - and heck, even toy code should have it.

Since you're using underscores in identifiers, I need to mention that it's important that this convention isn't applied to member names, especially public ones. This is simple code and there's no harm, but in more complex object-oriented code, an underscore in a procedure name can mean code that can't compile: VBA is reserving the underscore for a very specific meaning in member names.

---

I like that your function is pure, and therefore easily testable. But where are the tests?

With Rubberduck (a free, open-source VBE add-in I'm working on with a bunch of fellow VBA reviewers) you could document your requirements - and have tests that you can run anytime you make a change, as simple as this:

Option Explicit
Option Private Module

'@TestModule
'@Folder("Tests")

Private Assert As New Rubberduck.AssertClass

'@TestMethod
Public Sub AmericanStandardOutputContainsSlashSpacer()
    On Error GoTo TestFail

    Const splitter As String = " / "
    Dim actual As String

    actual = ConvertDateToFormattedCalendarWeek(Date, WFM_AmericanStandard)
    Assert.IsTrue InStr(expected, splitter) > 0

TestExit:
    Exit Sub
TestFail:
    Assert.Fail "Test raised an error: #" & Err.Number & " - " & Err.Description
End Sub

And so on:

• ISO standard weeks start on Monday
• American standard weeks start on Sunday
• ...

Every part of the specifications should have at least one test that covers it. And then you'll want tests to document behavior for invalid inputs, too:

'@TestMethod
Public Sub RaisesErrorGivenInvalidMode()
    Const ExpectedError As Long = 5
    On Error GoTo TestFail

    Const InvalidMode As Long = -1
    If InvalidMode = WFM_AmericanStandard Or InvalidMode = WFM_ISO8601 Then
        Assert.Inconclusive "InvalidMode must be an invalid parameter value."
    End If

    Dim actual As String
    actual = ConvertDateToFormattedCalendarWeek(Now, InvalidMode)

Assert:
    Assert.Fail "Expected error was not raised."

TestExit:
    Exit Sub
TestFail:
    If Err.Number = ExpectedError Then
        Resume TestExit
    Else
        Resume Assert
    End If
End Sub

Arguably, that "mode" parameter indicates that the function is doing too many things. If you split it into FormatDateAmericanStandard and FormatDateISO8601 there would be no ambiguity whatsoever about how a date is formatted when you look at the calling code. Compare to:

foo = convertDateToFormattedCalendarWeek(someDate, 0)

Is foo formatted per the American standard or ISO-8601? The enum certainly helps, but then if I have to pick between this:

foo = ConvertDateToFormattedCalendarWeek(someDate, WFM_AmericanStandard)
bar = ConvertDateToFormattedCalendarWeek(someDate, WFM_ISO8601)

And this:

foo = FormatDateAmericanStandard(someDate)
bar = FormatDateISO8601(someDate)

I pick the latter without hesitation - note how it completely eliminates the whole discussion about raising an error vs. returning "error", and eliminates the need to test/document what happens given an invalid mode parameter. And it makes the code much more explicit and self-descriptive, too.

A note about the name: you're not "converting a date" - you're "formatting a date" per a specified standard; I'd make the function name start with FormatDate.