Domain Name Service Class

I recently wrote a DSN class for use with the Access flavor of VBA. I'm preparing to refactor and would appreciate feedback.

I am aware of two issues.

• I added the findSectionKey() function to the Registry class (which is separate and out of scope for this question) that I call, but never went back to remove the duplicate logic from my DSN class. I am aware of this and will correct it during my refactor.

• I am completely unsatisfied with the way I implemented the Driver property. I use an Enum to specify the DriverType and then a select statement to return a string from the aforementioned Driver property. It's works well in practice, but is kind of clunky to work with (particularly when adding support for a new driver). I'm thinking I could use a Dictionary to do this better, but I've never used one and I'm unsure if that's the best approach.

So, I am concerned with Issue #2 first and foremost, but all feedback is welcome.

```
' *
' Class: DSN
' Author: Christopher J. McClellan
' Published under Creative Commons Attribution-Share Alike
' http://creativecommons.org/licenses/by-sa/3.0/
'
' Allows for easy creation of DSN entries in the ODBC Administrator by
' leveraging the Registry class.
' Entries in the ODBC Admin are not created until the
' Create() sub is called.
' *

Private Const CLASS_NAME As String = "DSN" 'for error handling

Public Enum eDSN_Bitmode
DSN_64BIT = 0
DSN_32BIT = 1
End Enum

Private Const REG_KEYNODE_32 As String = "software\wow6432Node\ODBC"
Private Const REG_KEYNODE_64 As String = "sof

This constant is not needed, and if you decided to rename the class to DomainNameService, it would be telling a lie:

Private Const CLASS_NAME As String = "DSN" 'for error handling

Instead, use TypeName(Me) to get the class' name as it appears at runtime.

The class has roughly 800 lines of code. Let's look at its public interface... side note, the inconsistently specified accessibility modifiers made me wonder what the default was in vba (if it's not specified, it's Public).

So:

Public Property Get Name() As String
Public Property Let Name(String) As String

Public Property Get Driver() As eDSN_Driver
Public Property Let Driver(eDSN_Driver)

Public Property Get DriverName() As String

Public Property Get DSNType() As eSDN_type
Public Property Let DSNType(eDSN_type)

Public Property Get BitMode() As eDSN_Bitmode
Public Property Let BitMode(eDSN_Bitmode)

Public Property Get DriverFile() As String

Public Property Get Description() As String
Public Property Let Description(String)

Public Property Get Server() As String
Public Property Let Server(String)

These "shared properties" are the properties of one type. The "Oracle properties" are members of another type, and the "SQL properties" are members of another type. I mean, I'd put them in 3 classes.

Using an Enum for Driver is an excellent idea, it makes you avoid using magic strings or worse, magic numbers. It makes sense to use a Select Case to switch on an enum:

Property Get DriverName() As String
' read only property
Select Case mDriver
    Case DSN_DRIVER_EMPTY
        DriverName = ""
    Case DSN_DRIVER_SQLSERVER
        DriverName = DSN_DRIVER_SQLSERVER_NAME
    Case DSN_DRIVER_SQLSERVER10
        DriverName = DSN_DRIVER_SQLSERVER10_NAME
    Case DSN_DRIVER_SQLSERVER11
        DriverName = DSN_DRIVER_SQLSERVER11_NAME
    Case DSN_DRIVER_ORA11G
        DriverName = DSN_DRIVER_ORA11G_NAME
    End Select
End Property

However I'd do it like this:

Property Get DriverName() As String

    Select Case mDriver            
        Case DSN_DRIVER_SQLSERVER
            DriverName = DSN_DRIVER_SQLSERVER_NAME

        Case DSN_DRIVER_SQLSERVER10
            DriverName = DSN_DRIVER_SQLSERVER10_NAME

        Case DSN_DRIVER_SQLSERVER11
            DriverName = DSN_DRIVER_SQLSERVER11_NAME

        Case DSN_DRIVER_ORA11G
            DriverName = DSN_DRIVER_ORA11G_NAME

        Case Else
            DriverName = vbNullString

    End Select

End Property

The difference? Vertical breathing whitespace, an explicit default value for anything that doesn't fit a Case block, and vbNullString being used instead of "". It's just a little technicality, but consider this - "" is not equivalent to vbNullString:

?lenb(vbnullstring), lenb("")
 0             0 
?strptr(vbnullstring), strptr("")
 0             56023156

If you really don't like maintaining a SELECT CASE, you could be maintaining a Dictionary instead (see this post for the implementation I'm referring to):

Private DriverNames As Dictionary

Private Sub InitializeDriverNames()

    DriverNames = New Dictionary ' this also works with a Scripting.Dictionary
    DriverNames.Add DSN_DRIVER_SQLSERVER, DSN_DRIVER_SQLSERVER_NAME
    DriverNames.Add DSN_DRIVER_SQLSERVER10, DSN_DRIVER_SQLSERVER10_NAME
    DriverNames.Add DSN_DRIVER_SQLSERVER11, DSN_DRIVER_SQLSERVER11_NAME
    DriverNames.Add DSN_DRIVER_ORA11G, DSN_DRIVER_ORA11_NAME

End Sub

And then you can add a call to InitializeDriverNames in Class_Initialize(), and the DriverName getter can look like this:

Public Property Get DriverName() As String
    Dim outResult As String
    If DriverNames.TryGetValue(mDriver, outResult) Then DriverName = outResult
    'If DriverNames.Exists(mDriver) Then DriverName = DriverNames.Item(mDriver)
End Property

The commented-out line shows how to make it work with a Scripting.Dictionary.

I like these two:

Private Sub ErrRaise_NotSupported()
    Err.Raise vbObjectError + 25010, CurrentProject.NAME & "." & CLASS_NAME, "Driver does not support the property."
End Sub

Private Sub ErrRaise_DriverNotSet()
    Err.Raise vbObjectError + 25020, CurrentProject.NAME & "." & CLASS_NAME, "Driver property is not set."
End Sub

However I'd write them like this:

Private Sub RaiseNotSupportedError()
    Err.Raise vbObjectError + 25010, CurrentProject.NAME & "." & TypeName(Me), "Driver does not support the property."
End Sub

Private Sub RaiseDriverNotSetError()
    Err.Raise vbObjectError + 25020, CurrentProject.NAME & "." & TypeName(Me), "Driver property is not set."
End Sub

And errBox should be called ShowErrorBox or similar, i.e. it should start with a verb, too.

Avoid comparing a Boolean with True or False like this:

If bool = True Then

boolAsString could be written like this:

Private Function boolAsString(ByVal value As Boolean) As String
    boolAsString = IIf(value, "T", "F")
End Function

IIf

StackExchange Code Review Q#51873, answer score: 7