HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

My script is about as scary as the XML it's reading

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptreadingthescaryxmlabout

Problem

This was a revision, made when the code failed after an update (at least that is what I think happened).

So I fixed it and now it works, but I think that this code can be written better and more efficiently, and perhaps made easier to read. Note that VBScript/VBA/VB6 isn't (aren't) my primary language(s), the closest thing I do is C#.

I presume that the If statements can be whittled down a little bit, (I know they can).

I am unsure of the version/flavor of VB this is, but given that it is written inside ` tags I presume it's VBScript.

``
Public Function GetParameterXml()
GetParameterXml = _
"" &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
" " &_
""
End Function

Dim Parameter: Set Parameter = Parameters.Item(Bookmark,"TYPE")
Dim Parameter2: Set Parameter2 = Parameters.Item(Bookmark,"NAMEFORMAT")
Dim sTemp
Dim oNodes: Set oNodes = Nothing
Dim oNode: Set oNode = Nothing
Dim PartyName()
Dim ArrayIndex
Dim varPartyName
Dim FirstName
Dim MidName
Dim LastName
Dim ParameterValue
Dim Suffix
ReturnData = ""

If Parameter2 is Nothing Then
ParameterValue = "1"
Else
ParameterValue = Parameter2.value
End If

ArrayIndex = 0

If Not Parameter is Nothing Then
Set oNodes = xmlDoc.selectNodes("/Record/CelloXml/Integration/Case/JudgmentEvent/Judgment/Additional/SDMonetaryAward/SDMonetaryAwardParties/SDMonetaryAwardParty[PartyConnection[@Word='"& uCase(parameter.Value) &"']]")

For each oNode in oNodes
For each oNode2 in oNode.childNodes
If oNode2.nodeName = "PartyName" Then
ReDim Preserve PartyName(ArrayIndex)
PartyName(ArrayIndex) = oNode2.text
ArrayIndex = ArrayIndex + 1
End If
Next
Next

For each varPartyName in PartyName

dim FirstMiddleName
LastName = Split(varPartyName, ",")(0)
FirstMiddleName = Trim(Split(varPartyName,",")(1))
Suffix = Trim(Split(VarPartyName,",")(2))

Solution

I'm assuming this is VBScript, and I'm not familiar at all with VBScript, and it seems pretty different from VBA/VB6 as far as iterating arrays (can't do For Each on an array in VBA/VB6) and string nullability is concerned - and VBA/VB6 isn't exactly crystal-clear as to what means what.

From this source (dates back from 2000, quotes a dead Microsoft link - emphasis & formatting mine):

-
"": A zero-length string (commonly called an "empty string") is technically a zero-length BSTR that actually uses six bytes of memory. In general, you should use the constant vbNullString instead, particularly when calling external DLL procedures.

-
Empty: A variant of VarType 0 (vbEmpty) that has not yet been initialized. Test whether it is "nil" using the IsEmpty function.

-
Nothing: Destroys an object reference using the Set statement. Test whether it is "nil" using the Is operator.

-
Null: A variant of VarType 1 (vbNull) that means "no valid data" and generally indicates a database field with no value. Don't confuse this with a C NULL, which indicates zero. Test whether it is "nil" using the IsNull function.

-
vbNullChar: A character having a value of zero. It is commonly used for adding a C NULL to a string or for filling a fixed-length string with zeroes.

-
vbNullString: A string having a value of zero, such as a C NULL, that takes no memory. Use this string for calling external procedures looking for a null pointer to a string. To distinguish between vbNullString and "", use the VBA StrPtr function: StrPtr(vbNullString) is zero, while StrPtr("") is a nonzero memory address.

So, it being a script, I'm not going to try to go and define classes here, although it seems to be supported. I'm still trying to wrap my head around all those conditions!

The conditions (and the fact that they are repeated in all main 4 branches) are actually hiding the intent.

So I would write a little Coalesce function and do away with all those checks:

Public Function Coalesce(ByVal value)
     If IsNull(value) Then
         Coalesce = vbNullString
     Else
         Coalesce = CStr(value)
     End If
 End Function


You seem to have a fixed amount of possible values for ParameterValue, to me this looks like a job for some Select Case rather than an If...ElseIf...ElseIf... block - also, separate the concerns of figuring out a line's content and of appending it to the result - because it's the ParameterValue that determines the order of the names, whether they're present or not doesn't matter, they can all be appended - as long as we ensure to replace Null values with non-value vbNullString:

'note: LastName can't ever be null... unless nothing was supplied at all.
Dim lineResult
Select Case ParameterValue

    Case "1": 
        'First Mid Last [Suffix]
        lineResult = Trim(Coalesce(FirstName) & " " _
                        & Coalesce(MidName) & " " _
                        & Coalesce(LastName) & " " _
                        & Coalesce(Suffix))

    Case "2": 
        'Last, First Mid [Suffix]
        lineResult = Trim(Coalesce(LastName) & ", " _
                        & Coalesce(FirstName) & " " _
                        & Coalesce(MidName) & " " _
                        & Coalesce(Suffix))

    Case "3": 
        'Last, First [Suffix] ~> Middle name ignored?
        lineResult = Trim(Coalesce(LastName) & ", " _
                        & Coalesce(FirstName) & " " _
                        & Coalesce(Suffix))

    Case "4": 
        'Last, First [Suffix], Mid
        lineResult = Trim(Coalesce(LastName) & ", " _
                        & Coalesce(FirstName) & " " _
                        & Coalesce(Suffix) & ", " _
                        & Coalesce(MidName))

End Select


This leaves us, in the worst cases, with a lineResult that's ended with ", ," or ",". Easy fix - not so neat, but hey between that and a bunch of nested If statements, I go with this:

If Right(lineResult, 2) = " ," Then lineResult = Left(lineResult, Len(lineResult) - 2)
If Right(lineResult, 1) = "," Then lineResult = Left(lineResult, Len(lineResult) - 1)


Now all we're missing is lineResult = lineResult & vbNewLine, and then we can do ReturnData = ReturnData & lineResult, and then we can iterate again.

Code Snippets

Public Function Coalesce(ByVal value)
     If IsNull(value) Then
         Coalesce = vbNullString
     Else
         Coalesce = CStr(value)
     End If
 End Function
'note: LastName can't ever be null... unless nothing was supplied at all.
Dim lineResult
Select Case ParameterValue

    Case "1": 
        'First Mid Last [Suffix]
        lineResult = Trim(Coalesce(FirstName) & " " _
                        & Coalesce(MidName) & " " _
                        & Coalesce(LastName) & " " _
                        & Coalesce(Suffix))

    Case "2": 
        'Last, First Mid [Suffix]
        lineResult = Trim(Coalesce(LastName) & ", " _
                        & Coalesce(FirstName) & " " _
                        & Coalesce(MidName) & " " _
                        & Coalesce(Suffix))

    Case "3": 
        'Last, First [Suffix] ~> Middle name ignored?
        lineResult = Trim(Coalesce(LastName) & ", " _
                        & Coalesce(FirstName) & " " _
                        & Coalesce(Suffix))

    Case "4": 
        'Last, First [Suffix], Mid
        lineResult = Trim(Coalesce(LastName) & ", " _
                        & Coalesce(FirstName) & " " _
                        & Coalesce(Suffix) & ", " _
                        & Coalesce(MidName))

End Select
If Right(lineResult, 2) = " ," Then lineResult = Left(lineResult, Len(lineResult) - 2)
If Right(lineResult, 1) = "," Then lineResult = Left(lineResult, Len(lineResult) - 1)

Context

StackExchange Code Review Q#33535, answer score: 6

Revisions (0)

No revisions yet.