2
\$\begingroup\$

This is the second review of these code blocks. I made major changes and updates to the functions during my refactoring and I would like a follow up review. The original review can be found here VBA Function Reads Active Live Data Feed and Translates Product Codes.

To reiterate: efficiency is absolutely key. I attempted to write some of the new support functions in a way that i can apply later in other pieces without having to rewrite them, which is why there are certain extra conditionals in the main code block. Any way to improve efficiency or refactor more or write cleaner better code I'm happy to hear!

Main Function Driver

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, horizontalExpiration 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(hedgeSplitArray) - 1) * 100)) & "d"

    ElseIf UCase(Mid(tradeLegStructureArray(0), 6, 1)) = "O" And UCase(Mid(tradeLegStructureArray(1), 6, 1)) = "O" Then
        'Two leg LIVE structures

        firstOptionLegArray() = Split(tradeLegStructureArray(0), ":")
        secondOptionLegArray() = Split(tradeLegStructureArray(1), ":")

        'different two leg structures
        If firstOptionLegArray(4) = secondOptionLegArray(4) Then
        'Call Spreads/Put Spreads

            assemblyString = "LIVE " & GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & BuildSpreadStructure(firstOptionLegArray, secondOptionLegArray)

        ElseIf firstOptionLegArray(4) <> secondOptionLegArray(4) Then
        'Straddle/Strangle/Fence

            If Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray))) = 0 Or _
            Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray))) <= -1 Then
            'fences

                assemblyString = "LIVE " & GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & BuildFenceStructure(firstOptionLegArray, secondOptionLegArray)

            ElseIf Val(firstOptionLegArray(UBound(firstOptionLegArray))) = Val(secondOptionLegArray(UBound(secondOptionLegArray))) Or _
            Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray))) >= 3 Then
            'straddle/strangle

                'Same strike straddle/differentstrike strangle
                If firstOptionLegArray(5) = secondOptionLegArray(5) Then
                'Straddle

                    assemblyString = GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & BuildStrangleStraddleStructures(firstOptionLegArray, secondOptionLegArray)

                ElseIf firstOptionLegArray(5) <> secondOptionLegArray(5) Then
                'strangle

                    assemblyString = "LIVE " & GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & BuildStrangleStraddleStructures(firstOptionLegArray, secondOptionLegArray)

                End If
            End If
        End If
    Else

        assemblyString = "Nothing"

    End If

    TwoLegStructureAnalysis = assemblyString

End Function

Support Functions

Public Function BuildStrangleStraddleStructures(firstOptionLegArray() As String, secondOptionLegArray() As String) As String

    Dim straddleBool As Boolean
    Dim assemblyString As String

    'Same strike straddle/Different strike strangle
    If firstOptionLegArray(5) = secondOptionLegArray(5) Then

        straddleBool = True

        If firstOptionLegArray(3) = secondOptionLegArray(3) Then
            'horizontal straddle when <> /rare case requires it's own conditional
            assemblyString = " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00")

        Else

            assemblyString = " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/" _
            & TranslateExpirationDate(secondOptionLegArray(3)) & " " & Format(secondOptionLegArray(5), "##0.00")

        End If


    ElseIf firstOptionLegArray(5) <> secondOptionLegArray(5) Then

        assemblyString = " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/"

        If firstOptionLegArray(3) = secondOptionLegArray(3) Then
            'Same expiration
            assemblyString = assemblyString & Format(secondOptionLegArray(5), "##0.00")

        Else
            'Horizontal
            assemblyString = assemblyString & TranslateExpirationDate(secondOptionLegArray(3)) & " " & Format(secondOptionLegArray(5), "##0.00")

        End If

    End If

    Select Case Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray)))
        'Checking for ratios on either structure
        Case 3 To 10

            assemblyString = assemblyString & " " & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & Abs(secondOptionLegArray(UBound(secondOptionLegArray)))

    End Select

    If straddleBool = True Then assemblyString = assemblyString & " Straddle" Else assemblyString = assemblyString & " Strangle"

    BuildStrangleStraddleStructures = assemblyString

End Function

Public Function BuildFenceStructure(firstOptionLegArray() As String, secondOptionLegArray() As String) As String
    'Builds Fence structures
    Dim assemblyString As String

    assemblyString = " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/"

    If firstOptionLegArray(3) = secondOptionLegArray(3) Then
        'Checks for horizontal expirations

        assemblyString = assemblyString & Format(secondOptionLegArray(5), "##0.00")

    Else
        'Horizontal
        assemblyString = assemblyString & TranslateExpirationDate(secondOptionLegArray(3)) & " " & Format(secondOptionLegArray(5), "##0.00")

    End If

    Select Case Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray)))
        'Handles Ratio

        Case -10 To -1

            assemblyString = assemblyString & " " & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & Abs(secondOptionLegArray(UBound(secondOptionLegArray)))

    End Select

    BuildFenceStructure = assemblyString & " Fence"

End Function

Public Function BuildSpreadStructure(firstOptionLegArray() As String, secondOptionLegArray() As String) As String
    'Builds call/put spreads
    Dim assemblyString As String
    Dim horizontalExpiration As Boolean

    'Same expirations
    If firstOptionLegArray(3) = secondOptionLegArray(3) Then

        assemblyString = TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & "/" & _
        Format(secondOptionLegArray(5), "##0.00")

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

            Case -10 To -1
                'Ratios
                assemblyString = assemblyString & " " & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & Abs(secondOptionLegArray(UBound(secondOptionLegArray)))

        End Select

    ElseIf firstOptionLegArray(3) <> secondOptionLegArray(3) Then
    'Horizontal

        horizontalExpiration = True

        assemblyString = 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 -10 To -1
            'Ratios
                assemblyString = assemblyString & " " & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & _
                Abs(secondOptionLegArray(UBound(secondOptionLegArray)))

            End Select

    End If

    'Determines and assigns call spread, put spread, rolls, etc
    'Returns completed structure analysis
    BuildSpreadStructure = assemblyString & " " & GetTypeOfSpread(GetCallOrPut(firstOptionLegArray(4)), horizontalExpiration, firstOptionLegArray(5), secondOptionLegArray(5))

End Function

Public Function GetTypeOfSpread(ByVal optionType As String, ByVal horizontalExpirations As Boolean, ByVal firstStrike As Double, ByVal secondStrike As Double) As String

    Dim spreadDesignation As String
    Dim optionsAreCalls As Boolean

    If optionType = "Call" Then optionsAreCalls = True

    If horizontalExpirations = False Then

        If optionsAreCalls = True Then spreadDesignation = "CS" Else spreadDesignation = "PS"

    Else

        If firstStrike = secondStrike Then

            If optionsAreCalls = True Then spreadDesignation = "Call Roll" Else spreadDesignation = "Put Roll"

        Else

            If optionsAreCalls = True Then spreadDesignation = "CS" Else spreadDesignation = "PS"

        End If

    End If

    GetTypeOfSpread = spreadDesignation

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

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 GetCallOrPut(ByVal legOption As String) As String
    'Translates C to Call and P to Put in option Structure

    Static map As Collection

    If map Is Nothing Then
        Set map = New Collection
        map.Add "Call", "C"
        map.Add "Put", "P"
    End If

    GetCallOrPut = map(legOption)

End Function

Public Function GetOptionStrike(ByVal tradeStructure As String, ByVal liveOptionBool As Boolean) As Double

    'Finds option strike within structure Code and separates it out.  Split
    Dim structureArray() As String

    structureArray() = Split(tradeStructure, ":", , vbTextCompare)

    Select Case liveOptionBool

        Case True

            GetOptionStrike = structureArray(UBound(structureArray))

        Case False

            GetOptionStrike = structureArray(UBound(structureArray) - 1)

    End Select

End Function


Public Function CountTradeLegSeparators(ByVal tradeStructure) As Integer

    Dim findChar As String, replaceChar As String

    findChar = "/"
    replaceChar = ""

    CountTradeLegSeparators = Len(tradeStructure) - Len(Replace(tradeStructure, findChar, replaceChar))

End Function
\$\endgroup\$
2
  • \$\begingroup\$ Quick question - is there a particular reason we should be aware of as to why your functions are public rather than private? \$\endgroup\$ Oct 6, 2016 at 20:08
  • 1
    \$\begingroup\$ There are functions in other modules that are accessing these functions in order to avoid redundant code in other modules. @Raystafarian \$\endgroup\$
    – StormsEdge
    Oct 6, 2016 at 21:03

1 Answer 1

1
\$\begingroup\$

I took a look over your support functions. First, I'm going to assume you are passing your array arguments ByRef on purpose.


BuildStrangleStraddleStructures

You use " " a lot, so why not create a constant for it and replace all instances of it with the constant?

Const WHITESPACE As String = " "

You have a lot of comments. Comments - "code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.

Your If block spacing is strange to me.

If firstOptionLegArray(3) = secondOptionLegArray(3) Then
            'horizontal straddle when <> /rare case requires it's own conditional
            assemblyString = WHITESPACE & TranslateExpirationDate(firstOptionLegArray(3)) & WHITESPACE & Format(firstOptionLegArray(5), "##0.00")

Else

            assemblyString = WHITESPACE & TranslateExpirationDate(firstOptionLegArray(3)) & WHITESPACE & Format(firstOptionLegArray(5), "##0.00") & "/" _
            & TranslateExpirationDate(secondOptionLegArray(3)) & WHITESPACE & Format(secondOptionLegArray(5), "##0.00")

End If

I think the blank lines aren't needed and don't add anything in terms of readability

    If firstOptionLegArray(3) = secondOptionLegArray(3) Then
        assemblyString = WHITESPACE & TranslateExpirationDate(firstOptionLegArray(3)) & WHITESPACE & Format(firstOptionLegArray(5), "##0.00")
    Else
        assemblyString = WHITESPACE & TranslateExpirationDate(firstOptionLegArray(3)) & WHITESPACE & Format(firstOptionLegArray(5), "##0.00") & "/" _
        & TranslateExpirationDate(secondOptionLegArray(3)) & WHITESPACE & Format(secondOptionLegArray(5), "##0.00")
    End If

Your check here

If firstOptionLegArray(5) = secondOptionLegArray(5) Then
ElseIf firstOptionLegArray(5) <> secondOptionLegArray(5) Then

You don't have any other possibilities, so that ElseIf can just be Else.

This only addresses cases 3 to 10 -

Select Case Val

What happens otherwise? Perhaps put in a Case Else to catch garbage or maybe just use an If to check the value instead, for simplicity

Dim ratio As Long
ratio = Val(firstOptionLegArray(UBound(firstOptionLegArray))) + Val(secondOptionLegArray(UBound(secondOptionLegArray)))
If ratio > 2 And ratio < 11 Then assemblyString = assemblyString & WHITESPACE & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & Abs(secondOptionLegArray(UBound(secondOptionLegArray)))

Here you check a boolean for TRUE

If straddleBool = True Then

Since it's boolean, no need for the = True -

If straddleBool then

All this applies to BuildFenceStructure and BuildSpreadStructure as well. You also have some other possibilities for constants with "/" and " Fence" as well as (possibly) "x"


GetTypeOfSpread

You have some more possibilities for these strings to be constants as well as eliminating If checking Boolean = True. Additionally, when you check for false like this

If horizontalExpirations = False Then

You can actually use

If Not horizontalExpirations Then

Also, it seems to me that the arguments for firstStrike and secondStrike aren't needed if you have the false horizontalExpirations, so those arguments in the function may actually be passed Optional ByVal and you can check if they are omitted to avoid all the extra checking.

I hate to bring up your If blocks again, but to me these are awkward:

    If firstStrike = secondStrike Then

        If optionsAreCalls = True Then spreadDesignation = "Call Roll" Else spreadDesignation = "Put Roll"

    Else

        If optionsAreCalls = True Then spreadDesignation = "CS" Else spreadDesignation = "PS"

    End If

They could be redone more simply

   If firstStrike = secondStrike Then
        If optionsAreCalls Then
            spreadDesignation = "Call Roll"
        Else: spreadDesignation = "Put Roll"
        End If
    Else
        If optionsAreCalls Then
            spreadDesignation = "CS"
        Else: spreadDesignation = "PS"
    End If

GetOptionCodes

I'm not sure if you're creating a static collection for a reason, but to me this could easily just be a Select Case.

The same can be said for TranslateExpirationDate and GetCallOrPut


I'd make ":" a constant called DELIMITER in GetOptionStrike and I'd give findChar a constant and get rid of replaceChar and use vbNullString in CountTradeLegSeparators. I'd also return a Long there instead of Integer - integers are obsolete. According to msdn VBA silently converts all integers to long.

\$\endgroup\$
2
  • \$\begingroup\$ I thought you weren't able to pass arrays ByVal? I attempted to do so and it gave me an error. \$\endgroup\$
    – StormsEdge
    Nov 1, 2016 at 15:59
  • \$\begingroup\$ You can only pass variant arrays ByVal, I believe. \$\endgroup\$ Nov 1, 2016 at 16:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.