patternMinor
VBA Function Reads Active Live Data Feed and Translates Product Codes
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
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
And then if you later need to map 42 to "W", all you need to add is
Ditto with
Now these lookups are \$O(1)\$ (instant) instead of \$O(n)\$ (worst-case you need to evaluate every
I'd do similar for
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
Then, divide & conquer: extract functions, one by one, each more and more specialized - and then you'll be looking at things like taking this:
And turning it into this:
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.
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 FunctionAnd 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 FunctionNow 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 SelectAnd 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 FunctionPublic 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 FunctionSelect 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 SelectDim 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.