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

VBA Function Reads Active Live Data Feed and Translates Product Codes

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

Problem

The following Function is one of the main analysis functions of a larger subroutine. The subroutine is responsible for translating codes that look like this "1|G|XNYM:O:LO:201611:P:44:+1/XNYM:O:LO:201611:C:51:+1" to something similar to this "LIVE WTI American X16 44.00/51.00 Strangle".

There are multiple scenarios and this is just one example, but I really wish to refactor this entire function to make it much more streamlined and clean. I struggled to combine conditionals and there are a lot of repetitions. How should I clean this up? The overall subroutine this is apart of is triggered by a worksheet change event, which can occur every few seconds; efficiency is absolutely key. Below the function I will post the various support functions this function makes calls to so it's clear what is going on.

Main Function

```
Public Function TwoLegStructureAnalysis(ByVal tradeStructure As String, ByVal liveOptionBool As Boolean) As String
'Trades with two legs analysis (two leg including hedged trades)
Dim tradeLegStructureArray() As String, hedgeSplitArray() As String, firstOptionLegArray() As String, secondOptionLegArray() As String
Dim assemblyString As String
Dim sameStrikeBool As Boolean

tradeLegStructureArray() = Split(tradeStructure, "/")

If UCase(Mid(tradeLegStructureArray(0), 6, 1)) = "O" And UCase(Mid(tradeLegStructureArray(1), 6, 1)) = "F" Then
'Hedged single Option trades

'Bifurcates the hedge by colon to split out delta and future
hedgeSplitArray() = Split(tradeLegStructureArray(1), ":")

assemblyString = GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & TranslateExpirationDate(Mid(tradeLegStructureArray(0), 11, 6)) _
& " " & Format(GetOptionStrike(tradeLegStructureArray(0), liveOptionBool), "##0.00") & " " & GetCallOrPut(Mid(tradeLegStructureArray(0), 18, 1)) & " x" & Format(hedgeSplitArray(UBound(hedgeSplitArray)), "##0.00") _
& " | " & Abs((hedgeSplitArray(UBound(h

Solution

The TranslateExpirationDate function looks like it could use a little map - a simple Static array that you gets initialized the first time the function is called:

Public Function TranslateExpirationDate(ByVal expirationDate As Double) As String

    Static map(1 To 12) As String
    If map(1) = vbNullString Then
        map(1) = "F"
        map(2) = "G"
        map(3) = "H"
        map(4) = "J"
        map(5) = "K"
        map(6) = "M"
        map(7) = "N"
        map(8) = "Q"
        map(9) = "U"
        map(10) = "V"
        map(11) = "X"
        map(12) = "Z"
    End If

    Dim integerPart As Integer
    integerPart = CInt(Right$(expirationDate, 2))

    TranslateExpirationDate = map(integerPart) & Mid$(expirationDate, 3, 2)

End Function


And then if you later need to map 42 to "W", all you need to add is map(42) = "W" and you're done - no need for a new Case block, no need to copy+paste anything.

Ditto with GetOptionCodes:

Public Function GetOptionCodes(ByVal optionType As String) As String
    Static map As Collection
    If map Is Nothing Then
        Set map = New Collection
        map.Add "WTI American", "LO"
        map.Add "HO American", "OH"
        map.Add "RB American", "OB"
        map.Add "NG European", "LN"
    End If
    GetOptionCodes = map(optionType)
End Function


Now these lookups are \$O(1)\$ (instant) instead of \$O(n)\$ (worst-case you need to evaluate every Case block to get your value) and as a bonus, you get stronger validation: if optionType isn't mapped, a runtime error occurs. In TranslateExpirationDate, if the integerPart is out of range, an index out of bounds runtime error occurs. The calling code should handle that.

I'd do similar for GetCallOrPut, so as to make sure something blows up in case of invalid input: it's better to blow up than keep running and produce trash output!

I've only glanced at the main procedure; at a glance, it seems like it's doing quite a lot of things - consider extracting each block into its own dedicated, more specialized function.

You'll want to use the strongly-typed string functions here (e.g. prefer Mid$ over Mid; Left$ over Left, Right$ over Right, UCase$ over UCase... see the whole list here), because the versions without the $ return a Variant that needs to be implicitly converted - if you're after performance, use the strongly-typed ones. Or stringly-typed, rather.

Then, divide & conquer: extract functions, one by one, each more and more specialized - and then you'll be looking at things like taking this:

Select Case Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray)))

    Case 2

        assemblyString = "LIVE " & GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/" & _
        TranslateExpirationDate(secondOptionLegArray(3)) & " " & Format(secondOptionLegArray(5), "##0.00") & " Strangle"

    Case 3 To 10

        assemblyString = "LIVE " & GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/" & _
        TranslateExpirationDate(secondOptionLegArray(3)) & " " & Format(secondOptionLegArray(5), "##0.00") & " " & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & Abs(secondOptionLegArray(UBound(secondOptionLegArray))) & " Strangle"

End Select


And turning it into this:

Dim result As String
result = "LIVE " & GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/" & _
        TranslateExpirationDate(secondOptionLegArray(3)) & " " & Format(secondOptionLegArray(5), "##0.00")

Select Case Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray)))
    Case 3 To 10
    result = result & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & Abs(secondOptionLegArray(UBound(secondOptionLegArray)))
End Select
result = result & " Strangle"


And then all that's missing is a little cleanup to reduce horizontal scrolling, and perhaps introduce a number of local variables to further reduce redundant function calls, and improve readability a bit more.

Code Snippets

Public Function TranslateExpirationDate(ByVal expirationDate As Double) As String

    Static map(1 To 12) As String
    If map(1) = vbNullString Then
        map(1) = "F"
        map(2) = "G"
        map(3) = "H"
        map(4) = "J"
        map(5) = "K"
        map(6) = "M"
        map(7) = "N"
        map(8) = "Q"
        map(9) = "U"
        map(10) = "V"
        map(11) = "X"
        map(12) = "Z"
    End If

    Dim integerPart As Integer
    integerPart = CInt(Right$(expirationDate, 2))

    TranslateExpirationDate = map(integerPart) & Mid$(expirationDate, 3, 2)

End Function
Public Function GetOptionCodes(ByVal optionType As String) As String
    Static map As Collection
    If map Is Nothing Then
        Set map = New Collection
        map.Add "WTI American", "LO"
        map.Add "HO American", "OH"
        map.Add "RB American", "OB"
        map.Add "NG European", "LN"
    End If
    GetOptionCodes = map(optionType)
End Function
Select Case Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray)))

    Case 2

        assemblyString = "LIVE " & GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/" & _
        TranslateExpirationDate(secondOptionLegArray(3)) & " " & Format(secondOptionLegArray(5), "##0.00") & " Strangle"

    Case 3 To 10

        assemblyString = "LIVE " & GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/" & _
        TranslateExpirationDate(secondOptionLegArray(3)) & " " & Format(secondOptionLegArray(5), "##0.00") & " " & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & Abs(secondOptionLegArray(UBound(secondOptionLegArray))) & " Strangle"

End Select
Dim result As String
result = "LIVE " & GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/" & _
        TranslateExpirationDate(secondOptionLegArray(3)) & " " & Format(secondOptionLegArray(5), "##0.00")

Select Case Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray)))
    Case 3 To 10
    result = result & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & Abs(secondOptionLegArray(UBound(secondOptionLegArray)))
End Select
result = result & " Strangle"

Context

StackExchange Code Review Q#142832, answer score: 2

Revisions (0)

No revisions yet.