VBA Class converter to eliminate strings being used as valid values

I'm trying to avoid strings being passed around as a valid value. The ability to make a mistake is too high for my taste and the solution that I've come up with is to use an Enum. This provides the benefit of intellisense when trying to use the values. Similar to typing sheet1.Visible = which gives the valid options a worksheet property with its type declared As SimpleSpanBracing allows the valid values be chosen. Now when I need to use something or to Get or Let a value I use the converter to ensure that I'm not going to mistakenly mix up casing, ends only or midspan, and accidentally get a false positive/negative.

Is there anything that I'm missing or haven't guarded against?

Enumeration with valid options. notSet is an option because I don't want to assume the default value. I'd prefer to see what's causing the issue and how it came about.

Public Enum SimpleSpanBracing
notSet
endsonly
midspan
thirdspan
End Enum


SpanBracingConverter Class that does the actual converting. I'm not concerned with getting a value that hasn't been set, but setting it to an invalid issues will cause an error to be raised, avoiding a false positive/negative mentioned above.

Private Const spanEnd As String = "ENDS ONLY"
Private Const spanMid As String = "MIDSPAN"
Private Const spanThird As String = "1/3 SPAN"
Private Const INVALID_INPUT As Long = 5

Public Function ToString(ByVal value As SimpleSpanBracing) As String
If value = endsonly Then
ToString = spanEnd
ElseIf value = midspan Then
ToString = spanMid
ElseIf value = thirdspan Then
ToString = spanThird
Else
Err.Raise INVALID_INPUT, "SpanBracingConverter.ToString()", "Invalid input supplied"
End If
End Function
Public Function ToEnum(ByVal value As String) As SimpleSpanBracing
value = UCase\$(value)
If value = spanEnd Then
ToEnum = endsonly
ElseIf value = spanMid Then
ToEnum = midspan
ElseIf value = spanThird Then
ToEnum = thirdspan
Else
Err.Raise INVALID_INPUT, "SpanBracingConverter.ToEnum()", "Invalid input supplied"
End If
End Function
Public Function ValidateString(ByVal value As String) As String
ValidateString = ToString(ToEnum(value))
End Function


What follows are the unit tests I've created with Rubberduck to test this converter.

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

'Arrange:
Dim sut As SpanBracingConverter
Set sut = New SpanBracingConverter

'Act:
Dim lowerEnd As SimpleSpanBracing
lowerEnd = sut.ToEnum("ends only")
Dim upperEnd As SimpleSpanBracing
upperEnd = sut.ToEnum("ENDS ONLY")
Dim lowerMid As SimpleSpanBracing
lowerMid = sut.ToEnum("midspan")
Dim upperMid As SimpleSpanBracing
upperMid = sut.ToEnum("MIDSPAN")
Dim lowerThird As SimpleSpanBracing
lowerThird = sut.ToEnum("1/3 span")
Dim upperThird As SimpleSpanBracing
upperThird = sut.ToEnum("1/3 SPAN")

Dim endInput As String
endInput = sut.ToString(endsonly)
Dim midInput As String
midInput = sut.ToString(midspan)
Dim thirdInput As String
thirdInput = sut.ToString(thirdspan)

'Assert:
Assert.areequal SimpleSpanBracing.endsonly, lowerEnd
Assert.areequal SimpleSpanBracing.endsonly, upperEnd
Assert.areequal SimpleSpanBracing.midspan, lowerMid
Assert.areequal SimpleSpanBracing.midspan, upperMid
Assert.areequal SimpleSpanBracing.thirdspan, lowerThird
Assert.areequal SimpleSpanBracing.thirdspan, upperThird

Assert.areequal "ENDS ONLY", endInput
Assert.areequal "MIDSPAN", midInput
Assert.areequal "1/3 SPAN", thirdInput

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

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

'Arrange:

'Act:
With New SpanBracingConverter
.ToEnum "fail"
End With

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

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

'Arrange:

'Act:
With New SpanBracingConverter
.ToString SimpleSpanBracing.notSet
End With

TestExit:
Exit Sub
TestFail:
If Err.Number = ExpectedError Then
Resume TestExit
Else
Assert.Fail "Expected error was not raised."
End If
End Sub

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

'Arrange:

'Act:
With New SpanBracingConverter
.ToString 4
End With

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

TestExit:
Exit Sub
TestFail:
If Err.Number = ExpectedError Then
Resume TestExit
Else
Assert.Fail "Expected error was not raised."
End If
End Sub

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

'Arrange:

'Act:

'Assert:
With New SpanBracingConverter
Assert.areequal "ENDS ONLY", .ValidateString("ENDS ONLY")
Assert.areequal "ENDS ONLY", .ValidateString("ends only")
Assert.areequal "MIDSPAN", .ValidateString("MIDSPAN")
Assert.areequal "MIDSPAN", .ValidateString("midspan")
Assert.areequal "1/3 SPAN", .ValidateString("1/3 SPAN")
Assert.areequal "1/3 SPAN", .ValidateString("1/3 span")
End With

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


Edit: My followup question, Part 2 continues my work on the converter class.

The Enum

An Enum is essentially a type, it has members. Members should be PascalCase - I'd change the alllowercase casing to respect that convention:

Public Enum SimpleSpanBracing
NotSet = 0
EndsOnly
MidSpan
ThirdSpan
End Enum


I would also make the NotSet value explicitly 0, because otherwise someone could very well think the enum would look prettier if all its members were listed alphabetically, and that would royally mess things up.

The Converter

The job of that class is essentially to map strings to enum values, and enum values to strings. Whenever I use the word "map", I automatically think of a Dictionary.

Picture your enum with 200 members. I doubt you find that If...ElseIf...ElseIf... conditional structure nice & neat now, huh?

A Select Case...Case...Case... block would be better IMO, but still rather annoying to maintain, given you essentially need the same blocks twice.

I'd go with a Private Dictionary - keyed with the strings and valued with the enums.

Then I'd have another Private Dictionary, keyed with [a string representation of] the enum values, and valued with the corresponding strings.

In the class' Initialize handler, I'd populate one of them, and then iterate it to automatically populate the other; of course the two methods would be implemented in their own dedicated Private Sub procedures, and the string constants would no longer serve a purpose.

Then, ToString and ToEnum would end up being as straightforward as it gets:

Public Function ToString(ByVal value As SimpleSpanBracing) As String
If Not StringForEnum.Exists(CStr(value)) Then
ThrowInvalidArgument "ToString", CStr(value)
'Exit Function
End If
ToString = StringForEnum(CStr(value))
End Function

Public Function ToEnum(ByVal value As String) As SimpleSpanBracing
If Not EnumForString.Exists(value) Then
ThrowInvalidArgument "ToEnum", value
'Exit Function
End If
ToEnum = EnumForString(value)
End Function


I'd have a dedicated ThrowInvalidArgument procedure responsible for throwing the invalid argument error, and move the INVALID_INPUT declaration there... or just remove it and inline the message string.

Private Sub ThrowInvalidArgument(ByVal source As String, ByVal value As String)
Err.Raise 5, TypeName(Me) & "." & source, _
"Invalid input '" & value & "' was supplied."
End Sub


The purpose of ValidateString doesn't strike me as super-obvious just by the name; my brain somehow expects "ValidateFoo" to either return a Boolean telling me whether I've supplied a valid Foo, or to throw an error given an invalid value. That method is really just a "round-trip" utility function that, IMO, should be inlined at call sites.

As a bonus of using that dictionary approach, you could expose a Property Get member that returns an array containing all the valid enum values, and another that returns all the valid strings. More on that in a minute.

The Tests

IMO all the Dim statements are cluttering the "Act" part of the tests and making it harder than necessary to read the actual "acting" executable statements.

Rubberduck's IAssert interface exposes a convenient SequenceEquals method that you can use to iterate and compare array elements; instead of making a series of Assert.AreEqual calls, you could have the converter expose a Property Get member that gives you all the valid strings as an array, a Property Get member that gives you all the valid enum values as another array, and then iterate one array to populate a "result" array by invoking your SUT/conversion in one single place (in that loop) - and then make one single Assert call to compare the expected vs. the actual resulting array.