# Handling name variants

Part of my standard modules deal with my company's employees and linking them to/from various data sets.

The following are a series of enums/functions to handle peoples' names, namely an Enum for advisers and functions to:

• Provide a collection of names associated to each adviser (e.g. their name before/after getting married)
• Provide the current name of each adviser
• Determine which adviser's name appears in a given string

Is this a good, maintainable, extensible system? Especially considering the possibility of new employees over time?

Option Explicit

MartinCotter = 1
JonHussey = 2
JeremySmith = 3
JonathanBlair = 4
JohnCusins = 5
SarahCotter = 6
MickyMahbubani = 7
End Enum

Dim coll As Collection
Set coll = New Collection

Case JonHussey

Case MartinCotter

Case JeremySmith

Case JonathanBlair

Case SarahCotter

Case JohnCusins

Case MickyMahbubani

End Select

End Function

Public Sub AddNameVariations(ByRef coll As Collection, ByVal forename As String, ByVal surname As String)

coll.Add forename & " " & surname
coll.Add surname & ", " & forename

End Sub

Dim nameString As String

Case JonHussey
nameString = "Hussey, Jon"

Case MartinCotter
nameString = "Cotter, Martin"

Case JeremySmith
nameString = "Smith, Jeremy"

Case JonathanBlair
nameString = "Blair, Jonathan"

Case SarahCotter
nameString = "Oluwole, Sarah"

Case JohnCusins
nameString = "Cusins, John"

Case MickyMahbubani
nameString = "Mahbubani, Micky"

End Select

End Function

Else

End If

End Function

Dim nameColl As Collection

Dim i As Long
For i = 1 To nameColl.Count
If InStr(1, str, nameColl(i)) > 0 Then
Exit For
End If
Next i

End Function


First, I thought I'd mention that your code is stylistically really good - variable and function names make sense, indentation is great (with one small exception noted below), and the functions are focused and readable. Just one minor point:

The code under each Case in a Select Case is typically indented one more level:

Select Case adviser
Case JonHussey
Case MartinCotter
'...


As far as your specific question goes, this actually looks like it would be quite a chore to maintain. Adding a new name to an existing advisor is easy with your set-up. For example, if John Cusins decided to change his name to Jim Nasium, all you would have to do is add AddNameVariations coll, "Jim", "Nasium" under his Case in NameStringsFromAdviser and change his Case in CurrentNameStringFromAdviser to NameString = "Nasium, Jim".

But, this points to the next issue - your Enum isn't really serving much use as much other than a place-holder. On the one hand you could change the JohnCusins element to JimNasium everywhere in your code, but this kind of defeats the purpose of having a named identifier. On the other hand you could leave it as JohnCusins, but this now means that it isn't appropriately named - why should the Enum element associated with a person be different from their name? If I was only looking at the CurrentNameStringFromAdviser function and not the entire module, I'd be wondering "why is the SarahCotter case returning Oluwole, Sarah?"

This also points to the main issue in maintainability and extensibility - to add an advisor, you need to change the code in 3 different functions and add a value to the enumeration. At the same time, you need to make sure that those changes are all consistent with each other. The urge to copy and paste will be very hard to resist...

...which leads to the last point - you're repeating code all over the place. This gives me a whiff of code smell. Wouldn't it be nice if you didn't have to type If AdviserNameInString(FooBar, str) Then again and again and again? It would be much better to approach this problem with the mantra Don't Repeat Yourself in mind. This is a problem begging for an OO solution.

You have 2 distinct things you care about - advisors and names. So make an advisor class and a name class. Make them responsible for the things that advisors and names care about. I.e.:

Name.cls

Option Explicit

Private mFirst As String
Private mLast As String

Public Property Let FirstName(first As String)
mFirst = first
End Property

Public Property Get FirstName() As String
FirstName = mFirst
End Property

Public Property Let LastName(last As String)
mLast = last
End Property

Public Property Get LastName() As String
LastName = mLast
End Property

Public Property Get NameString() As String
NameString = mLast & ", " & mFirst
End Property

Public Function Matches(test As String) As Boolean
Matches = InStr(1, test, mFirst) Or InStr(1, test, mLast)
End Function


Option Explicit

Private mNames As New Collection
Private mPreferred As Name

End Sub

Public Property Set PreferredName(inValue As Name)
Set mPreferred = inValue
End Property

Public Property Get PreferredName() As Name
Set PreferredName = mPreferred
End Property

Public Function NameMatches(test As String) As Boolean
Dim candidate As Variant
For Each candidate In mNames
If candidate.Matches(test) Then
NameMatches = True
Exit For
End If
Next candidate
End Function


Note, it isn't clear from your question if the numbers in the Enum have meaning outside of the code that you posted. If they are, just make them a property of Advisor. Throw in a couple factory functions to easily generate objects (curse you VBA for not having constructors)...

'Name factory.
Public Function NewName(first As String, last As String) As Name
Set NewName = New Name
NewName.FirstName = first
NewName.LastName = last
End Function

Public Function NewAdvisor(preferred As Name, _
ParamArray names() As Variant) As Advisor
Dim variation As Variant

For Each variation In names
Dim cast As Name
Set cast = variation
Next variation
End Function


...and you get something to maintain that looks like this:

Option Explicit

NewName("Sarah", "Cotter"))
End Sub

Dim candidate As Variant
If candidate.NameMatches(inValue) Then
Exit Function
End If
Next candidate
End Function


Example usage:

Private Sub ExampleUsage()