4
\$\begingroup\$

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(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))

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

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

                    Case 0
                    'No ratio

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

                    Case Else

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

                End Select


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

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

                    Case 0
                    'again no ratio

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

                    Case Else
                    'Ratios

                        assemblyString = assemblyString & " " & 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)))

                    End Select

            End If

            'Determines callspread or Put Spread
            If GetCallOrPut(firstOptionLegArray(4)) = "Call" Then assemblyString = assemblyString & " CS" Else assemblyString = assemblyString & " PS"
'''''''''''''''
        ElseIf firstOptionLegArray(4) <> secondOptionLegArray(4) Then
        'Straddle/Strangle/Fence

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


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

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

                        Case 0
                        'No ratio

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

                        Case -1 To -10
                        'Ratio

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

                    End Select


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

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

                        assemblyString = GetOptionCodes(Mid(tradeLegStructureArray(0), 8, 2)) & " " & TranslateExpirationDate(firstOptionLegArray(3)) & " " & Format(firstOptionLegArray(5), "##0.00") & " Straddle"

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

                        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") & "/" & _
                                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") & "/" & _
                                Format(secondOptionLegArray(5), "##0.00") & " " & Abs(firstOptionLegArray(UBound(firstOptionLegArray))) & "x" & Abs(secondOptionLegArray(UBound(secondOptionLegArray))) & " Strangle"

                        End Select

                    End If
                End If


            'Horizontal/Different Expirations
            ElseIf firstOptionLegArray(3) <> secondOptionLegArray(3) Then

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

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

                        Case 0
                        'No ratio

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

                        Case -1 To -10
                        'Ratio

                            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))) & " Fence"

                    End Select


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

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

                        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
                    End If
                End If
            End If
          End If
        Else

            assemblyString = "Nothing"

        End If

    TwoLegStructureAnalysis = assemblyString

End Function

Support Functions

Public Function GetOptionCodes(ByVal optionType As String) As String

    Select Case UCase(optionType)

        Case "LO"

            GetOptionCodes = "WTI American"

        Case "OH"

            GetOptionCodes = "HO American"

        Case "OB"

            GetOptionCodes = "RB American"

        Case "LN"

            GetOptionCodes = "NG European"

    End Select

End Function

Public Function TranslateExpirationDate(ByVal expirationDate As Double) As String

    Select Case CInt(Right(expirationDate, 2))

        Case 1

            TranslateExpirationDate = "F" & Mid(expirationDate, 3, 2)

        Case 2

            TranslateExpirationDate = "G" & Mid(expirationDate, 3, 2)

        Case 3

            TranslateExpirationDate = "H" & Mid(expirationDate, 3, 2)

        Case 4

            TranslateExpirationDate = "J" & Mid(expirationDate, 3, 2)

        Case 5

            TranslateExpirationDate = "K" & Mid(expirationDate, 3, 2)

        Case 6

            TranslateExpirationDate = "M" & Mid(expirationDate, 3, 2)

        Case 7

            TranslateExpirationDate = "N" & Mid(expirationDate, 3, 2)

        Case 8

            TranslateExpirationDate = "Q" & Mid(expirationDate, 3, 2)

        Case 9

            TranslateExpirationDate = "U" & Mid(expirationDate, 3, 2)

        Case 10

            TranslateExpirationDate = "V" & Mid(expirationDate, 3, 2)

        Case 11

            TranslateExpirationDate = "X" & Mid(expirationDate, 3, 2)

        Case 12

            TranslateExpirationDate = "Z" & Mid(expirationDate, 3, 2)

    End Select

End Function

Public Function GetCallOrPut(ByVal legOption As String) As String
    'Translates C to Call and P to Put in option Structure

    If legOption = "C" Then

        GetCallOrPut = "Call"

    ElseIf legOption = "P" Then

        GetCallOrPut = "Put"

    End If

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\$

1 Answer 1

Reset to default
2
\$\begingroup\$

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.

\$\endgroup\$
4
  • \$\begingroup\$ I have a question; when do you decide something should be a separate function? I had been considering making a whole bunch of functions, but my fear was that I would be overdoing it. What are your thoughts on that? \$\endgroup\$ Commented Sep 30, 2016 at 12:00
  • 1
    \$\begingroup\$ You can't really "overdo" coding by the single responsibility principle ...okay perhaps you can, but the point is, distinct functions imply distinct concerns, or responsibilities - I decide to move something elsewhere when the abstraction levels become unclear in a procedure that includes high-level DoSomething and then nitty gritty details of how a set of columns on some worksheet is formatted. If you can put a comment saying "this chunk of code does x", then that chunk belongs in its own method, with a meaningful name instead of a comment. \$\endgroup\$ Commented Sep 30, 2016 at 12:10
  • \$\begingroup\$ Got it! That's very useful thank you! One follow up question; how should these be organized? Should I have a separate module for support functions and such? Or does that not so much matter? \$\endgroup\$ Commented Sep 30, 2016 at 12:39
  • 1
    \$\begingroup\$ What you want to achieve is cohesion: the members of a given module should be closely related - if you extract the support functions into another module, and they're not used anywhere else because they're actually too specialized... then you're not improving anything really, the support functions would be better off as private functions in the module that consumes them. \$\endgroup\$ Commented Sep 30, 2016 at 13:43

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.