I have recently come to use VBA class modules in my code.
Based in part on an answer on Stack Overflow, I bought VBA Developer's Handbook and Professional Excel Development, but it took me a long time to 'get' classes (though I still have a long way to go).
I wrote myself a GetExternalData
class, as one of the very common tasks I undertake in Excel VBA is incorporating data from other sources into my reports.
My thought was that the class could take care of deciding the best way of getting the data, based on the source and how the data is to be used, so my getExternalData
method was using a 'Variant' so that I could use the same variable whether I return a Collection
, Array
, Dictionary
or Worksheet
.
However, when I attempted to pass the Collection to another function which expects a collection I was getting an error. I noticed the locals window showed as 'Variant/Object/Collection', so I tried setting the value to an Object rather than a Variant which works.
My question on Stack Overflow was actually about this practice (Dim as Object... set = Collection
).
TODO:
At the moment, I don't have anything coded for Access database connections
I check the first element of a collection, and assume all items are similar types and have the same number of elements (for arrays)
I know there's debate on the merits of Hungarian - at the moment, I find it useful (have been using it for a long time in Access (tbl, qry) and in VBA user-forms (txt, cmb), so implementing it into my VBA made sense - so I don't really want to get into that debate
There's also mixed opinion on accessors vs public variables. If I create a letter then I create a getter, except where I'm storing the return Cols in a collection
Option Explicit
Public Enum dataReturnType
arrayVals
collectionVals
dictionaryVals
End Enum
Public Enum sourceFileType
csv = 1 '
xls = 10 'old Excel
mdb 'old Access
xlsx = 100 'new Excel
xlsm 'new Excel macro-enabled
accdb 'new Access
End Enum
Private Enum getDataMethod
ADO_Record_Set
Web_QT
End Enum
Private strFilePath As String
Private strFileName As String
Private enumFileType As sourceFileType
Private strConnString As String
Private strExtendedProperties As String
Private strSelectClause As String
Private strFromClause As String
Private colReturnFields As Collection
Private enumRetrievalMethod As getDataMethod
Private strFormatString As String
Private boolIMEX As Boolean
Private boolHDR As Boolean
Private intkeyCol As Integer
Private rngOutput as Range
Public WSname As String
Public WhereClause As String
Public RtnType As dataReturnType
Private Sub Class_Initialize()
boolHDR = True
boolIMEX = True
Set colReturnFields = New Collection
strSelectClause = "*"
End Sub
Property Let FilePath(fullPathOnly As String)
If Left(fullPathOnly, 4) = "http" Then
enumRetrievalMethod = Web_QT
If Right(fullPathOnly, 1) = "/" Then strFilePath = fullPathOnly Else strFilePath = fullPathOnly & "/"
Else
enumRetrievalMethod = ADO_Record_Set
If Right(fullPathOnly, 1) = "\" Then strFilePath = fullPathOnly Else strFilePath = fullPathOnly & "\"
End If
End Property
Property Get FilePath() As String
FilePath = strFilePath
End Property
Property Let FileName(fName As String)
strFileName = fName
Select Case Right(strFileName, Len(strFileName) - InStr(strFileName, "."))
Case Is = "txt", "csv", "lst"
enumFileType = csv
Case Is = "xls"
enumFileType = xls
Case Is = "xlsx", "xlsb"
enumFileType = xlsx
Case Is = "xlsm"
enumFileType = xlsm
Case Is = "mdb"
enumFileType = mdb
Case Is = "accdb"
enumFileType = accdb
End Select
End Property
Property Get FileName() As String
FileName = strFileName
End Property
Property Let ReturnFields(colNums As Variant)
Dim colNum As Variant
Select Case VarType(colNums)
Case Is < vbVariant
colReturnFields.Add colNums
Case Else
For Each colNum In colNums
colReturnFields.Add colNum
Next
End Select
End Property
Property Let externalDataKeyColumn(colNum As Integer)
intkeyCol = colNum
End Property
Property Get externalDataKeyColumn() As Integer
externalDataKeyColumn = intkeyCol
End Property
Public Function getExternalData() As Variant
Dim ReturnVals As Object
Dim DataProcessor As Object
Set DataProcessor = New DataProcessor
Select Case enumRetrievalMethod
Case Is = getDataMethod.ADO_Record_Set
Set ReturnVals = doConnectRS
Select Case Me.RtnType
Case Is = dataReturnType.arrayVals
getExternalData = DataProcessor.collection2Array(ReturnVals, Me.externalDataKeyColumn, True)
Case Is = collectionVals
Set getExternalData = ReturnVals
Case Is = dictionaryVals
Set getExternalData = New scripting.Dictionary
Set getExternalData = DataProcessor.collection2Dictionary(ReturnVals, Me.externalDataKeyColumn)
End Select
Case Is = getDataMethod.Web_QT
Set ReturnVals = doConnectQT
Select Case Me.RtnType
Case Is = dataReturnType.arrayVals
getExternalData = ReturnVals.UsedRange
Case Is = collectionVals
Set getExternalData = DataProcessor.array2Collection(ReturnVals.UsedRange, Me.externalDataKeyColumn)
Case Is = dictionaryVals
Set getExternalData = DataProcessor.array2Dictionary(ReturnVals.UsedRange, Me.externalDataKeyColumn)
End Select
End Select
End Function
Private Function doConnectRS() As Collection
'RecordSet Constants
Const adOpenStatic = 3
Const adLockOptimistic = 3
Const adCmdText = &H1
Dim dp As New DataProcessor
Dim objRecordSet As Object, objConnection As Object
Dim varRS As Variant
Dim connString As String, SqlStatement As String
Dim colNum As Integer, rowNum As Integer
Dim colRS As New Collection
Dim keyCol As Variant
If Not compatibleSource Then Exit Function
connString = getConnectionProperties
SqlStatement = "Select " & strSelectClause & " FROM " & strFromClause
If WhereClause <> vbNullString Then SqlStatement = SqlStatement & " WHERE " & WhereClause
Set objConnection = CreateObject("ADODB.Connection")
Set objRecordSet = CreateObject("ADODB.Recordset")
objConnection.Open connString
objRecordSet.Open SqlStatement, objConnection, adOpenStatic, adLockOptimistic, adCmdText
If colReturnFields.Item(1) = "all" Then
colReturnFields.Remove (1)
For colNum = 1 To objRecordSet.Fields.Count
colReturnFields.Add colNum
Next
End If
Do Until objRecordSet.EOF
ReDim varRS(1 To colReturnFields.Count)
For colNum = 1 To colReturnFields.Count
If IsNull(objRecordSet.Fields.Item(CLng(colReturnFields.Item(colNum) - 1))) Then
varRS(colNum) = vbNullString 'If the SQL statement returns Null populate with vbNullString rather than Null
Else
varRS(colNum) = objRecordSet.Fields.Item(CLng(colReturnFields.Item(colNum) - 1))
End If
Next
colRS.Add varRS
objRecordSet.MoveNext
Loop
Set objRecordSet = Nothing
Set objConnection = Nothing
Set doConnectRS = colRS
End Function
Private Function compatibleSource() As Boolean
Select Case enumFileType
Case Is > xlsx
If Application.Version < 12 Then
MsgBox "Incompatible source file selected!", vbCritical
compatibleSource = False
Else
compatibleSource = True
End If
Case Else
compatibleSource = True
End Select
End Function
Private Function getConnectionProperties()
Dim HDR As String
If boolHDR Then HDR = "HDR=YES" Else HDR = "HDR=NO"
Select Case enumFileType
Case Is = xls
strConnString = "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & strFilePath & strFileName & ";"
strExtendedProperties = "Extended Properties=""Excel 8.0;"
strFromClause = "[" & WSname & "$]"
Case Is = csv
strConnString = "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & strFilePath & ";"
strExtendedProperties = "Extended Properties=""text; FMT=Delimited;"
strFromClause = strFileName
Case Is = xlsx
strConnString = "Provider=Microsoft.ACE.OLEDB.12.0;Data Source=" & strFilePath & strFileName & ";"
strExtendedProperties = "Extended Properties=""Excel 12.0 Xml;"
strFromClause = "[" & WSname & "$]"
Case Is = xlsm
strConnString = "Provider=Microsoft.ACE.OLEDB.12.0;Data Source=" & strFilePath & strFileName & ";"
strExtendedProperties = "Extended Properties=""Excel 12.0 Macro;"
strFromClause = "[" & WSname & "$]"
End Select
getConnectionProperties = strConnString & strExtendedProperties & HDR & ";"
If boolIMEX Then getConnectionProperties = getConnectionProperties & "IMEX=1;"
getConnectionProperties = getConnectionProperties & """"
End Function
Private Function doConnectQT() As Worksheet
Dim ws As Worksheet
Dim qT As QueryTable
If rngOutput Is Nothing Then
Set ws = Worksheets.Add
Set rngOutput = ws.Range("a1")
Else
Set ws = rngOutput.Parent
End If
myFileName = "URL; " & Me.FilePath & Me.FileName
Set qT = ws.QueryTables.Add(Connection:=myFileName, Destination:=rngOutput)
With qT
.Name = "myQT"
.fieldNames = True
.RowNumbers = False
.FillAdjacentFormulas = False
.PreserveFormatting = True
.RefreshOnFileOpen = False
.BackgroundQuery = True
.RefreshStyle = xlInsertDeleteCells
.SavePassword = False
.SaveData = True
.AdjustColumnWidth = True
.RefreshPeriod = 0
.WebSelectionType = xlEntirePage
.WebFormatting = xlWebFormattingNone
.WebPreFormattedTextToColumns = True
.WebConsecutiveDelimitersAsOne = True
.WebSingleBlockTextImport = False
.WebDisableDateRecognition = False
.WebDisableRedirections = False
.Refresh BackgroundQuery:=False
End With
qT.Delete
Set doConnectQT = ws
End Function
For the sake of completeness, I will also post the 'DataProcessor' class which I use to convert the returned collection to a dictionary or array if needed
Public Function collection2Array(colIn As Collection, Optional ByVal keyCol As Integer, Optional writeOut As Boolean) As Variant
Dim rowCount As Long, colCount As Long
Dim arrOut As Variant
Select Case VarType(colIn.Item(1))
Case Is >= vbVariant 'if the collection contains arrays
ReDim arrOut(1 To colIn.Count, 1 To UBound(colIn.Item(1))))
For rowCount = 1 To UBound(colIn.Item(1))
For colCount = 1 To UBound(arrOut, 2)
arrOut(rowCount, colCount) = colIn.Item(rowCount)(colCount)
Next
Next
Case Else
If writeOut Then 'we'll return a 2D array with 1 column suitable for writing directly to a worksheet
ReDim arrOut(1 To colIn.Count, 1 To 1)
For rowCount = 1 To colIn.Count
arrOut(rowCount, 1) = colIn.Item(rowCount)
Next
Else
ReDim arrOut(1 To colIn.Count)
For rowCount = 1 To colIn.Count
arrOut(rowCount) = colIn.Item(rowCount)
Next
End If
End Select
collection2Array = arrOut
End Function
Public Function collection2Dictionary(colIn As Collection, Optional ByVal keyCol As Integer, Optional compareMode As VbCompareMethod = vbTextCompare) As scripting.Dictionary
Dim colVal As Variant
Set collection2Dictionary = New scripting.Dictionary
collection2Dictionary.compareMode = compareMode
If keyCol = 0 Then keyCol = 1
For Each colVal In colIn
collection2Dictionary.Item(colVal(keyCol)) = colVal
Next
End Function
1 Answer 1
GetExternalData
isn't ideal for a class name; if methods are verbs, classes/types are nouns. Class names should be nouns, not verbs.
When making a class, unless the class' primary concern is presentation, I leave MsgBox
and other notifications out of it, and raise custom runtime errors instead.
This one caught my eye:
Property Let FileName(fName As String) strFileName = fName Select Case Right(strFileName, Len(strFileName) - InStr(strFileName, ".")) Case Is = "txt", "csv", "lst" enumFileType = csv Case Is = "xls" enumFileType = xls Case Is = "xlsx", "xlsb" enumFileType = xlsx Case Is = "xlsm" enumFileType = xlsm Case Is = "mdb" enumFileType = mdb Case Is = "accdb" enumFileType = accdb End Select End Property
If the extension is unknown, you'll have inconsistent internal state in your object. If the file name contains dots (e.g. some.file.with.dots.csv
), there's a bug in your Case
expression that will leave enumFileType
unassigned. Consider using the much more robust FileSystemObject
to retrieve a file's extension, instead of implementing your own:
Private Function GetFileExtension(ByVal path As String)
With New Scripting.FileSystemObject
GetFileExtension = .GetExtensionName(path)
End With
End Function
Also, Case Is =
is rather uncommon; this would be equivalent:
Property Let FileName(ByVal value As String)
Dim extension As String
extension = GetFileExtension(value)
Select Case extension
Case "txt", "csv", "lst"
enumFileType = csv
Case "xls"
enumFileType = xls
Case "xlsx", "xlsb"
enumFileType = xlsx
Case "xlsm"
enumFileType = xlsm
Case "mdb"
enumFileType = mdb
Case "accdb"
enumFileType = accdb
Case Else
Err.Raise 5, TypeName(Me), "File format '." & extension & "' is not supported."
End Select
strFileName = value
End Property
Notice the Case Else
that prevents assigning strFileName
with a value that we don't know what to do with, by raising a runtime error.
The casing isn't consistent; in VBA public members should be PascalCase
. I'm not going to say a word about Hungarian notation, except "ugh" (that's not a word, is it?).
There's also mixed opinion on accessors vs public variables.
Not really. A public field in a standard module is a global variable, and that's pretty much universally recognized as a bad practice whenever avoidable. A public field in a class module breaks encapsulation, which literally defeats the purpose of a class module. In every single object-oriented language, a public field is bad practice. How is VBA any different? Avoid public fields. Period.
What's up with the indentation in doConnectQT
?
End With qT.Delete Set doConnectQT = ws End Function
The indentation of getConnectionProperties
isn't consistent with the rest of the module either. I've copied your code into a class module and Rubberduck cleanly indented the whole thing in a split second.
Speaking of Rubberduck, here's what its code inspections have to say:
Warning: Constant 'adOpenStatic' is implicitly Variant - CR14196VBA.ExternalData, line 134
Warning: Constant 'adLockOptimistic' is implicitly Variant - CR14196VBA.ExternalData, line 135
Warning: Constant 'adCmdText' is implicitly Variant - CR14196VBA.ExternalData, line 136
Suggestion: Public field 'WSname' breaks encapsulation - CR14196VBA.ExternalData, line 37
Suggestion: Public field 'WhereClause' breaks encapsulation - CR14196VBA.ExternalData, line 38
Suggestion: Public field 'RtnType' breaks encapsulation - CR14196VBA.ExternalData, line 39
Warning: Member 'Worksheets' implicitly references ActiveWorkbook - CR14196VBA.ExternalData, line 221
Hint: Member 'FilePath' is implicitly public - CR14196VBA.ExternalData, line 48
Hint: Member 'FilePath' is implicitly public - CR14196VBA.ExternalData, line 58
Hint: Member 'FileName' is implicitly public - CR14196VBA.ExternalData, line 62
Hint: Member 'FileName' is implicitly public - CR14196VBA.ExternalData, line 80
Hint: Member 'ReturnFields' is implicitly public - CR14196VBA.ExternalData, line 84
Hint: Member 'externalDataKeyColumn' is implicitly public - CR14196VBA.ExternalData, line 96
Hint: Member 'externalDataKeyColumn' is implicitly public - CR14196VBA.ExternalData, line 99
Suggestion: Move module-level variable 'strConnString' to a smaller scope. - CR14196VBA.ExternalData, line 26
Suggestion: Move module-level variable 'strExtendedProperties' to a smaller scope. - CR14196VBA.ExternalData, line 27
Suggestion: Move module-level variable 'rngOutput' to a smaller scope. - CR14196VBA.ExternalData, line 36
Suggestion: Move module-level variable 'WSname' to a smaller scope. - CR14196VBA.ExternalData, line 37
Suggestion: Move module-level variable 'WhereClause' to a smaller scope. - CR14196VBA.ExternalData, line 38
Suggestion: Move module-level variable 'RtnType' to a smaller scope. - CR14196VBA.ExternalData, line 39
Warning: Instruction contains multiple declarations - CR14196VBA.ExternalData, line 138
Warning: Instruction contains multiple declarations - CR14196VBA.ExternalData, line 140
Warning: Instruction contains multiple declarations - CR14196VBA.ExternalData, line 141
Suggestion: Consider renaming enum member 'csv' - CR14196VBA.ExternalData, line 10
Suggestion: Consider renaming enum member 'xls' - CR14196VBA.ExternalData, line 11
Suggestion: Consider renaming enum member 'mdb' - CR14196VBA.ExternalData, line 12
Suggestion: Consider renaming enum member 'xlsx' - CR14196VBA.ExternalData, line 13
Suggestion: Consider renaming enum member 'xlsm' - CR14196VBA.ExternalData, line 14
Suggestion: Consider renaming variable 'dp' - CR14196VBA.ExternalData, line 137
Suggestion: Consider renaming variable 'HDR' - CR14196VBA.ExternalData, line 190
Suggestion: Consider renaming variable 'ws' - CR14196VBA.ExternalData, line 217
Suggestion: Consider renaming variable 'qT' - CR14196VBA.ExternalData, line 218
Warning: Return value of function 'getExternalData' is never used. - CR14196VBA.ExternalData, line 103
Hint: Parameter 'fullPathOnly' is implicitly passed by reference - CR14196VBA.ExternalData, line 48
Hint: Parameter 'fName' is implicitly passed by reference - CR14196VBA.ExternalData, line 62
Hint: Parameter 'colNums' is implicitly passed by reference - CR14196VBA.ExternalData, line 84
Hint: Parameter 'colNum' is implicitly passed by reference - CR14196VBA.ExternalData, line 96
Hint: Return type of member 'getConnectionProperties' is implicitly 'Variant' - CR14196VBA.ExternalData, line 189
Suggestion: Parameter 'fullPathOnly' can be passed by value - CR14196VBA.ExternalData, line 48
Suggestion: Parameter 'fName' can be passed by value - CR14196VBA.ExternalData, line 62
Suggestion: Parameter 'colNums' can be passed by value - CR14196VBA.ExternalData, line 84
Suggestion: Parameter 'colNum' can be passed by value - CR14196VBA.ExternalData, line 96
Warning: function 'getExternalData' is not used - CR14196VBA.ExternalData, line 103
Hint: Object reference 'dp' is self-assigned - CR14196VBA.ExternalData, line 137
Hint: Object reference 'colRS' is self-assigned - CR14196VBA.ExternalData, line 142
Warning: Variable 'strFormatString' is never assigned - CR14196VBA.ExternalData, line 32
Warning: Variable 'WSname' is never assigned - CR14196VBA.ExternalData, line 37
Warning: Variable 'WhereClause' is never assigned - CR14196VBA.ExternalData, line 38
Warning: Variable 'RtnType' is never assigned - CR14196VBA.ExternalData, line 39
Warning: Variable 'rowNum' is never assigned - CR14196VBA.ExternalData, line 141
Warning: Variable 'keyCol' is never assigned - CR14196VBA.ExternalData, line 143
Warning: variable 'strFormatString' is not used - CR14196VBA.ExternalData, line 32
Warning: variable 'dp' is not used - CR14196VBA.ExternalData, line 137
Warning: variable 'rowNum' is not used - CR14196VBA.ExternalData, line 141
Warning: variable 'keyCol' is not used - CR14196VBA.ExternalData, line 143
Suggestion: Property 'ReturnFields' has no getter - CR14196VBA.ExternalData, line 84
Now, some of these inspection results are popping up only because I have no client code to consume the class, call its members and assign its public fields, but ignoring that, you can see that there are a number of serious issues with this code - variables declared but never used, ReturnFields
property being write-only (consider making it a method instead), members are implicitly Public
, parameters are implicitly passed ByRef
when they could very well be passed ByVal
, some variables have cryptic 2-3 letter names, etc.
I'm not sure I like how you're passing arbitrary concatenated SQL to an ADODB connection - the WHERE clause should already be part of the query, and parameters should be ?
question marks, and your code would be taking in actual parameter values (say, using a ParamArray
parameter, or an array of values) and creating ADODB Parameter
objects and executing an ADODB Command
instead of concatenating arbitrary SQL.
</SqlSecurityRant>
I think replacing Null
values with vbNullString
should be some setting that the client code can decide about: "no value" is semantically very different from "empty value".
I'm not sure why you're late-binding ADODB here. You lose IntelliSense, take a performance hit at runtime, and you wouldn't need to redefine the adEnum constants.
In getConnectionProperties
you're using the function's return value as if it were a local variable:
getConnectionProperties = strConnString & strExtendedProperties & HDR & ";"
If boolIMEX Then getConnectionProperties = getConnectionProperties & "IMEX=1;"
getConnectionProperties = getConnectionProperties & """"
Avoid this, it hurts readability and looks like a recursive call. And declare the function As String
- right now it's implicitly returning a Variant
- there's so much Variant
and Object
in this code, you have implicit type conversions through the roof!
I'd like to point out this inspection result:
Hint: Object reference 'colRS' is self-assigned - CR14196VBA.ExternalData, line 142
Consider this code:
Public Sub DoSomething()
Dim foo As New Collection
foo.Add "test1"
Set foo = Nothing
foo.Add "test2" ' runtime error? isn't foo supposed to be Nothing?
End Sub
When using As New
at procedure scope like you do here:
Dim colRS As New Collection
...you're changing how VBA treats that object's lifetime, in a way that can introduce subtle and surprising bugs. Avoid As New
at procedure scope.
It's hard to tell module-scope variables from globals. Adepts of Hungarian notation usually annotate their private fields with some m_
prefix for that purpose. I have a better way (granted, that T
prefix is totally Hungarian - as is the I
prefix I use on all my interface classes... Hungarian does have a number of benefits... just not in variable naming):
Private Type TClassName
Foo As String
Bar As Integer
'...
End Type
Private this As TClassName
Public Property Get Foo() As String
Foo = this.Foo
End Property
Public Property Let Foo(ByVal value As String)
this.Foo = value
End Property
Public Property Get Bar() As Integer
Bar = this.Bar
End Property
Public Property Let Bar(ByVal value As Integer)
this.Bar = value
End Property
'...
By wrapping all your private fields in a Private Type
, you make it idiot-proof what the class' fields are, and you have one single field (this
) to refer to everywhere in the class - that way whenever you read the code and see anything prefixed with this.
, you know you can only be looking at a private field.
It also eliminates the casing/naming problem: the properties and their corresponding underlying private field names can match without breaking any naming convention, and without having to resort to funky m_
prefixes.
As for the relevance of Hungarian notation (sorry!), when your IDE tells you what the type of a variable is, what purpose does the prefix serve?
colReturnFields (variable: Collection
Meaningful names and consistent naming conventions are much more useful than Hungarian notation: I know I'm looking at a collection, or group of things, simply because Fields
is plural - the col
prefix isn't really useful at all.
enumRetrievalMethod (variable: getDataMethod
If I wasn't paying attention, I'd think getDataMethod
is a Function
or some member/procedure; the enum type name should simply not start with a verb - it's a type, therefore it's a noun.
I guess that's a wrap :)
(also: avoid underscores in identifier names - spare your future self headaches, stick to PascalCase
for types and their members, and camelCase
for locals and parameters)
-
1\$\begingroup\$ Thanks for the comment. You're entirely right about having a CASE ELSE for unknown filetypes, and I'm pleased to learn about Scripting.FileSystemObject for retrieving file extensions, much better than finding the period in the name! As for your code inspections, I'm not sure what to make of most of that. Could you provide me with some info on your
this
technique? I don't immediately see the benefit. As for Hungarian, "when your IDE tells you what the type of a variable is [in break mode]", it can be useful to know at design-time, too! "strConnoteNumber" leaves no doubt that it's alphanumeric \$\endgroup\$emjaySX– emjaySX2016年09月24日 03:03:28 +00:00Commented Sep 24, 2016 at 3:03 -
\$\begingroup\$ "I know I'm looking at a collection, or group of things, simply because Fields is plural - the col prefix isn't really useful at all" - it's useful to me to know whether the data-type is a Dictionary, or a Collection, or an Array (since collections and dictionaries have the same methods, context doesn't help). And you're right about the MsgBox - that's a hold-over from some testing, and should have already been removed! \$\endgroup\$emjaySX– emjaySX2016年09月24日 03:15:59 +00:00Commented Sep 24, 2016 at 3:15
-
1\$\begingroup\$ Try Ctrl+i on any variable at design time... these RD screenshots are at design time, too. As for the inspections, they identify unused variables, parameters passed
ByRef
that can/should beByVal
, members that are implicitlyPublic
that could use an explicit access modifier, things declared without an explicit type that are implicitlyVariant
, etc. The benefits of thethis
field are already in the answer - you'll never clash with a global variable if you do that, and it's easier to tell a field from a local or parameter too. Not to mention instant serialization if you need it. \$\endgroup\$Mathieu Guindon– Mathieu Guindon2016年09月24日 13:24:16 +00:00Commented Sep 24, 2016 at 13:24 -
1\$\begingroup\$ I know this is not a forum, but this is a great opportunity for me to actually learn. It was this article on Stack Overflow that convinced me accessors are unnecessary if all you're doing is storing and returning in a 'dumb container': stackoverflow.com/questions/1568091/why-use-getters-and-setters. I don't see that
Ctrl+i
is more convenient thancol
*. Finally, my intention is to move everything to late binding, as my organisation has Excel from XP to 2010 still in use (which is why I do the version validation in the class) \$\endgroup\$emjaySX– emjaySX2016年09月24日 23:53:53 +00:00Commented Sep 24, 2016 at 23:53 -
1\$\begingroup\$ Odd, that SO page is presenting overwhelming evidence and explanation about why public fields are a bad idea and why exposing class data as properties is better. If that won't convince you, I don't know what will. Your class is not a "dumb data holder" (that's exactly what a
Public Type
is; a structure exposing a bunch of public fields and nothing more), it embeds functionality and presents an interface - arguably a number of these fields should simply not be exposed at all. \$\endgroup\$Mathieu Guindon– Mathieu Guindon2016年09月25日 00:01:40 +00:00Commented Sep 25, 2016 at 0:01
Integer
is not worth the clutter. Knowing that something is an array index, though, or a combo box, that's more useful. I recommend the excellent Joel Spolsky post on the topic: joelonsoftware.com/articles/Wrong.html \$\endgroup\$strConnoteNumber
andstrRunNo
leave no ambiguity as to what data-type they actually are \$\endgroup\$