patternMinor
Retrieving ListBox.column values and store into objects
Viewed 0 times
objectscolumnintoandlistboxstorevaluesretrieving
Problem
I know there must be a better way to perform the above sub than using multiple
.
.
.
Refactored:
NB: wellObj has all properties type
If statements. I was thinking of storing the obj.PropertName into a collection and run it through a for loop.'Create instance
Dim wellObj As CWell
Set wellObj = New CWell.
.
.
Private Sub Well_List_Click()
'Must check if null..
If Not (Well_List.Column(1, row) = "") Then
wellObj.wellName = Well_List.Column(1, row)
End If
If Not (Well_List.Column(2, row) = "") Then
wellObj.wellActive = Well_List.Column(2, row)
End If
If Not (Well_List.Column(3, row) = "") Then
wellObj.wellDiameter = Well_List.Column(3, row)
End If
If Not (Well_List.Column(4, row) = "") Then
wellObj.wellCasing = Well_List.Column(4, row)
End If
If Not (Well_List.Column(5, row) = "") Then
wellObj.screenedInterval = Well_List.Column(5, row)
End If
If Not (Well_List.Column(6, row) = "") Then
wellObj.wellSock = Well_List.Column(6, row)
End If
If Not (Well_List.Column(7, row) = "") Then
wellObj.wellDepth = Well_List.Column(7, row)
End If
End SubRefactored:
NB: wellObj has all properties type
StringPrivate Sub Well_List_Click()
Dim properties(1 To 7) As String
Dim value As String
Dim row As Integer
'Skip header
row = Well_List.ListIndex + 1
'Must check if null..
properties(1) = "WellName"
properties(2) = "WellActive"
properties(3) = "WellDiameter"
properties(4) = "wellCasing"
properties(5) = "ScreenedInternal"
properties(6) = "WellSock"
properties(7) = "WellDepth"
For i = 1 To 7
value = Well_List.Column(i, row)
If value <> vbNullString Then
CallByName wellObj, properties(i), VbLet, value
End If
Next
End SubSolution
Start with proper indentation. There shouldn't be anything (other than subroutine/line labels) at the same level of indentation as a
Code blocks are more readable and code as a whole is easier to follow when code blocks are indented. General rule of thumb, if there's an
Using a
Now you can go from 1 to 7 and, assuming
Notice I'm using
So, how would that
This is assuming
That said, public members should be
Now, I really introduced that
Private Sub statement, other than the End Sub.Code blocks are more readable and code as a whole is easier to follow when code blocks are indented. General rule of thumb, if there's an
End X for a statement, you're in a code block. Give that Tab key some lovin'!Private Sub DoSomething()
|...If Not FooBar Then
|...|...DoSomethingElse
|...End If
End Sub DoSomethingUsing a
For..Next loop here is tricky, because you're accessing a different property every time. You're going to need to access the properties by name, and to do that you need them in an indexed structure, like an array:Dim properties(1 To 7) As String
properties(1) = "wellName"
properties(2) = "wellActive"
properties(3) = "wellDiameter"
properties(4) = "wellCasing"
properties(5) = "screenedInternal"
properties(6) = "wellSock"
properties(7) = "wellDepth"Now you can go from 1 to 7 and, assuming
CWell is a class module (I'm not all that familiar with Access, I hope my assumption is correct)...For i = 1 To 7
value = Well_List.Column(i, row)
If value <> vbNullString Then SetPropertyValue wellObj, properties(i), value
NextNotice I'm using
vbNullString instead of an empty string literal "". vbNullString is a null string pointer, which isn't allocated a memory address. "" has its own memory spot. No biggie, but not needed either.So, how would that
SetPropertyValue method be implemented? The VBA standard library has this little wonder in its VBA.Interaction module, called CallByName:Private Sub SetPropertyValue(ByRef instance, ByVal member, ByVal value)
CallByName instance, member, vbLet, value
End SubThis is assuming
CWell exposes a Property Let accessor/mutator. If wellName and friends are public fields, you need to properly encapsulate them and expose them via properties.That said, public members should be
PascalCase, so "wellName" would be "WellName".Now, I really introduced that
SetPropertyValue method just to make an abstraction level for the sake of this answer. The one-liner could just as well be inlined:Dim properties(1 To 7) As String
properties(1) = "wellName"
properties(2) = "wellActive"
properties(3) = "wellDiameter"
properties(4) = "wellCasing"
properties(5) = "screenedInternal"
properties(6) = "wellSock"
properties(7) = "wellDepth"
For i = 1 To 7
value = Well_List.Column(i, row)
If value <> vbNullString Then
CallByName wellObj, properties(i), vbLet, value
End If
NextCode Snippets
Private Sub DoSomething()
|...If Not FooBar Then
|...|...DoSomethingElse
|...End If
End Sub DoSomethingDim properties(1 To 7) As String
properties(1) = "wellName"
properties(2) = "wellActive"
properties(3) = "wellDiameter"
properties(4) = "wellCasing"
properties(5) = "screenedInternal"
properties(6) = "wellSock"
properties(7) = "wellDepth"For i = 1 To 7
value = Well_List.Column(i, row)
If value <> vbNullString Then SetPropertyValue wellObj, properties(i), value
NextPrivate Sub SetPropertyValue(ByRef instance, ByVal member, ByVal value)
CallByName instance, member, vbLet, value
End SubDim properties(1 To 7) As String
properties(1) = "wellName"
properties(2) = "wellActive"
properties(3) = "wellDiameter"
properties(4) = "wellCasing"
properties(5) = "screenedInternal"
properties(6) = "wellSock"
properties(7) = "wellDepth"
For i = 1 To 7
value = Well_List.Column(i, row)
If value <> vbNullString Then
CallByName wellObj, properties(i), vbLet, value
End If
NextContext
StackExchange Code Review Q#94342, answer score: 8
Revisions (0)
No revisions yet.