2
\$\begingroup\$

My original question got an answer that helped me revise my code. Below is updated code with a converter that has a few more choices.

These enum members are prefaced with ss since numbers aren't allowed as starting characters.

'SectionSetConverter Class
Public Enum SectionSets
    NotSet
    ss14x6
    ss12x8
    ss12x6
    ss10x10
    ss10x8
    ss10x6
    ss8x8
    Aggregate
    FullList
End Enum

'@Folder("Converters")
Option Explicit

Private StringForEnum As Dictionary
Private EnumForString As Dictionary

Private Sub Class_Initialize()
    PopulateDictionaries
End Sub

Private Sub PopulateDictionaries()
    Set EnumForString = New Dictionary
    EnumForString.CompareMode = TextCompare

    EnumForString.Add "14x6", ss14x6
    EnumForString.Add "12x8", ss12x8
    EnumForString.Add "12x6", ss12x6
    EnumForString.Add "10x10", ss10x10
    EnumForString.Add "10x8", ss10x8
    EnumForString.Add "10x6", ss10x6
    EnumForString.Add "8x8", ss8x8
    EnumForString.Add "Aggregate", Aggregate
    EnumForString.Add vbNullString, FullList

    Set StringForEnum = New Dictionary
    EnumForString.CompareMode = TextCompare
    Dim i As Variant
    For Each i In EnumForString.Keys
        StringForEnum.Add EnumForString.Item(i), i
    Next
End Sub

Public Function ToEnum(ByVal value As String) As SectionSets
    value = UCase$(value)

    If Not EnumForString.Exists(value) Then
        ThrowInvalidArgument "ToEnum", value
    End If

    ToEnum = EnumForString(value)
End Function

Public Function ToString(ByVal value As SectionSets)
    If Not StringForEnum.Exists(value) Then
        ThrowInvalidArgument "ToString", CStr(value)
    End If

    ToString = StringForEnum(value)
End Function

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

Public Property Get Enums() As Variant
    Enums = EnumForString.Items
End Property

Public Property Get Strings() As Variant
    Strings = EnumForString.Keys
End Property

Note: The annotation '@Folder("Converters") helps organize all my converters into a "Folder" for easier viewing with the Code Explorer of the RubberduckVBA add-in.


Following the advice of @MathieuGuindon I created the private Dictionary (Requires early reference to be set in the VBE by going to Tools>References>Microsoft Scripting Runtime) for each of the values.

There's now only a single list to maintain. The population of the the dictionary and iteration of the Keys to populate the second list guarantees that they won't become out of sync and will reflect the correct Key Value Pair (KVP).

The above exposed functions ToEnum and ToString convert the parameter to the correct value. Any invalid information causes ThrowInvalidArgument to be called and raise the appropriate error.

Lastly the properties that allow for simpler unit tests, which coincidentally follow.

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

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

    Dim converterEnums As Variant
    converterEnums = sut.Enums

    Dim converterStrings As Variant
    converterStrings = sut.Strings

    Dim actualStrings As Variant
    ReDim actualStrings(LBound(converterStrings) To UBound(converterStrings))

    Dim actualEnums As Variant
    ReDim actualEnums(LBound(converterStrings) To UBound(converterStrings))

    'Act:
    Dim i As Long
    For i = LBound(actualStrings) To UBound(actualStrings)
        actualStrings(i) = sut.ToString(converterEnums(i))
    Next

    For i = LBound(actualEnums) To UBound(actualEnums)
        actualEnums(i) = sut.ToEnum(converterStrings(i))
    Next

    'Assert:
    Assert.sequenceequals converterStrings, actualStrings
    Assert.sequenceequals converterEnums, actualEnums

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

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

    'Arrange:

    'Act:
    With New SectionSetConverter
        .ToString SectionSets.NotSet
    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 LargestAssignedEnumValue()
    Const ExpectedError As Long = 5
    On Error GoTo TestFail

    'Arrange:

    'Act:
    With New SectionSetConverter
        .ToString 10
    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

Previously the test ValidInputs had an awkward feel to it. Adding members to SectionSet would have required me to update the unit test every time. Now if a member is added their respective valid values are already tested thanks to the Enums and Strings properties on SectionSetConverter.

Are there any other parts I can improve on? I've thought about creating an IConverter class and having each converter Implements it but am undecided on the benefits of doing so presently.

\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

I'd go with the constant vbTextCompare for your CompareMode on your objects so a user can simply choose if they want to use early or late binding.

Mostly, I am confused about the Enum. When we initialize we end up with -

StringForEnum as Dictionary
StringForEnum(1) = 1
StringForEnum(2) = 2
StringForEnum(3) = 3
StringForEnum(4) = 4
StringForEnum(5) = 5
StringForEnum(6) = 6
StringForEnum(7) = 7
StringForEnum(8) = 8
StringForEnum(9) = 9

Enums as Variant
Enums(0) = 1
Enums(1) = 2
Enums(2) = 3
Enums(3) = 4
Enums(4) = 5
Enums(5) = 6
Enums(6) = 7
Enums(7) = 8
Enums(8) = 9

EnumForString as Dictionary
EnumForString(1) = "14x6"
EnumForString(2) = "12x8"
EnumForString(3) = "12x6"
EnumForString(4) = "10x10"
EnumForString(5) = "10x8"
EnumForString(6) = "10x6"
EnumForString(7) = "8x8"
EnumForString(8) = "Aggregate"
EnumForString(9) = ""

Strings as Variant
Strings(0) = "14x6"
Strings(1) = "12x8"
Strings(2) = "12x6"
Strings(3) = "10x10"
Strings(4) = "10x8"
Strings(5) = "10x6"
Strings(6) = "8x8"
Strings(7) = "Aggregate"
Strings(8) = ""

Right, but your properties Enums and Strings don't have a Let method, which means it cannot be changed while running.

You also notice that your variants start at 0 and your dictionaries start at 1. So why are these duplicated in such a way that the Dictionary is needed, considering it requires a reference?

So, I'm not sure, but I'll assume the integer references you want start at 1 instead of 0. So this all can be accomplished with just one property and one constant -

Const STRING_VALUES = "14x6,12x8,12x6,10x10,10x8,10x6,8x8,Aggregate,"

Private Strings As Variant

Public Sub Class_Initialize()
    Strings = Split(STRING_VALUES, ",")
End Sub

Public Property Get ToString(value As Long)
    ToString = Strings(value - 1)
End Property

I also don't know why you would need to expose a property that returns the entire variable array - isn't that why it's in a Class? So you don't need to store it in a different object?

Maybe I'm missing the point, but this all seems a bit overkill.

\$\endgroup\$
1
  • \$\begingroup\$ I don't expose any Let property since I don't intend for them to be changed when running. I hadn't though of the 0/1 difference in variant and dictionary. Although that's taken care of with the NotSet enumeration option. I'll look into implementing your shorthand string version and see if it grows on me. \$\endgroup\$ Commented Apr 16, 2018 at 23:26

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.