I would like to make sure I'm using good practices for my first major project. When creating Get Properties, is it better to use methods to change multiple private variables and return objects in classes different that the class in which a property is defined?
Here is a (working) specific excerpt from a Class I developed:
Is (READ-ONLY) Public Property Get AllTradesBySpread(Bounds_ASD() As Integer, Optional SubCollection_ASD As Collection = Nothing) As Collection best-implemented as a method? It works, but I'm beginning to wonder if some of the properties I'm writing should be methods.
Although the following code is long, only due to repetition, it's well-commented. It only manipulates two private variables (pNTradesInTradesBySpread & pAllTradesBySpread); however, many of my properties manipulate several.
Public Property Get AllTradesBySpread(Bounds_ASD() As Integer, Optional SubCollection_ASD As Collection = Nothing) As Collection
'/CLASS: CCollectTrades
'/PURPOSE: organize all Option Contract trades (stored as CTradeSpread custom-defined objects) into embedded collections...
'/...by returning a fixed number (14) of collections w/ embedded collections plus item 1, where...
'/...the first item in the collection is an array containing all spreads/descriptions, in an order defined below...
'/...from pEveryTrade (if SubCollection_ASD is omitted or Nothing) or SubCollection_ASD
'/INPUT: Bounds_ASD() is the input to embedded Property: EveryTrade(Bounds_ASD), described below
'/Bounds_ASD() input details:
'/Bounds_ASD() Pupose: data validation and to set limits of For loops when storing option trades as CTradeSpread objects
'/Bounds_ASD() Input: Workbook applicable to Bounds
'/Bounds_ASD() Output: an array of integers (of fixed UBound = 7), containing the bounds of the trade history we're operating on
'/CBStart = BoundsArray(0) -- Cash Balance
'/CBEnd = BoundsArray(1)
'/AOHStart = BoundsArray(2) -- Account Order History
'/AOHEnd = BoundsArray(3)
'/ATHStart = BoundsArray(4) -- Account Trade History
'/ATHEnd = BoundsArray(5)
'/LastRow = BoundsArray(6)
'/LastColumn = BoundsArray(7)
'/OPTIONAL INPUT: SubCollection_ASD, a collection of CTradeSpread objects that may be organized into spreads if we wish to organize...
'/...a set of trades other than pEveryTrade
'/OUTPUT: A collection of trades w/ spreads ordered as such:
'/Item 1: An array with each element a description of the spread (dev. needed for 4-leg butterfly)
'/Item 2: SINGLE
'/Item 3: VERTICAL
'/Item 4: CALENDAR
'/Item 5: DIAGONAL
'/Item 6: STRADDLE
'/Item 7: STRANGLE
'/Item 8: BUTTERFLY (3-leg)
'/Item 9: BUTTERFLY (4-LEG)
'/Item 10: IRON CONDOR
'/Item 11: ~IRON CONDOR
'/Item 12: CUSTOM
'/Item 13: STOCK
'/Item 14: VERT ROLL
'/Item 15: OTHER
'/Embedded Property: Public Property Get EveryTrade(Bounds() As Integer) As Collection
'/CLASS: CCollectTrades
'/Purpose: Create a collection of all option spread trades that are contained w/i a spreadsheet
'/Input: Bounds of WB (specifies where location of datapoints trades such as Cash Balance, Account Trade History, etc.)
'/Output: A collection of Every CTradeSpread (user-defined object) on a Worksheet referenced by Bounds()
'/Note: looping over all trades once is more efficient than using SpreadCollection property b/c the loop...
'/...traverses pEveryTrade (or SubCollection) once; whereas, the SpreadCollection property...
'/...traverses pEveryTrade (or SubCollection) once per spread
'/Variables
Dim i_ASD As Integer, NAllTrades_ASD As Integer
Dim Spread_ASD As String
'/Array
Dim SpreadList() As String
'/Objects
Dim Output_ASD As Collection
Set Output_ASD = New Collection
Dim TradesToCatagorize_ASD As Collection
Set TradesToCatagorize_ASD = New Collection
Dim SINGLEs_ASD As Collection
Set SINGLEs_ASD = New Collection
Dim VERTICALs_ASD As Collection
Set VERTICALs_ASD = New Collection
Dim CALENDARs_ASD As Collection
Set CALENDARs_ASD = New Collection
Dim DIAGONALs_ASD As Collection
Set DIAGONALs_ASD = New Collection
Dim STRADDLEs_ASD As Collection
Set STRADDLEs_ASD = New Collection
Dim STRANGLEs_ASD As Collection
Set STRANGLEs_ASD = New Collection
Dim BUTTERFLY3s_ASD As Collection
Set BUTTERFLY3s_ASD = New Collection
Dim BUTTERFLY4s_ASD As Collection
Set BUTTERFLY4s_ASD = New Collection
Dim IRONCONDORs_ASD As Collection
Set IRONCONDORs_ASD = New Collection
Dim sIRONCONDORs_ASD As Collection
Set sIRONCONDORs_ASD = New Collection
Dim CUSTOMs_ASD As Collection
Set CUSTOMs_ASD = New Collection
Dim OTHERs_ASD As Collection
Set OTHERs_ASD = New Collection
Dim STOCKs_ASD As Collection
Set STOCKs_ASD = New Collection
Dim VERTROLLs_ASD As Collection
Set VERTROLLs_ASD = New Collection
If pAllTradesBySpread Is Nothing Then Set pAllTradesBySpread = New Collection
If SubCollection_ASD Is Nothing Then
If pEveryTrade Is Nothing Then
Set pEveryTrade = EveryTrade(Bounds_ASD)
End If
Set TradesToCatagorize_ASD = pEveryTrade
Else
Set TradesToCatagorize_ASD = SubCollection_ASD
End If
NAllTrades_ASD = TradesToCatagorize_ASD.Count
If NAllTrades_ASD > 0 Then
pNTradesInTradesBySpread = 0
For i_ASD = 1 To NAllTrades_ASD
Spread_ASD = TradesToCatagorize_ASD.Item(i_ASD).Spread
Select Case Spread_ASD
Case "SINGLE"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
SINGLEs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case "VERTICAL"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
VERTICALs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case "CALENDAR"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
CALENDARs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case "DIAGONAL"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
DIAGONALs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case "STRADDLE"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
STRADDLEs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case "STRANGLE"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
STRANGLEs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD) 'UNTESTED
Case "BUTTERFLY"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
BUTTERFLY3s_ASD.Add TradesToCatagorize_ASD.Item(i_ASD) '3 LEG
Case "BUTTERFLY4L"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
BUTTERFLY4s_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case "IRON CONDOR"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
IRONCONDORs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case "~IRON CONDOR"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
sIRONCONDORs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case "CUSTOM"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
CUSTOMs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD) 'UNTESTED
Case "STOCK"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
STOCKs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case "VERT ROLL"
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
VERTROLLs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
Case Else
pNTradesInTradesBySpread = pNTradesInTradesBySpread + 1
OTHERs_ASD.Add TradesToCatagorize_ASD.Item(i_ASD)
End Select
Next i_ASD
Else
MsgBox ("Error in Class CCollectTrades, Property AllSpreads: collection to sort is empty")
End If
ReDim SpreadList(13)
SpreadList(0) = "SINGLE"
SpreadList(1) = "VERTICAL"
SpreadList(2) = "CALENDAR"
SpreadList(3) = "DIAGONAL"
SpreadList(4) = "STRADDLE"
SpreadList(5) = "STRANGLE"
SpreadList(6) = "BUTTERFLY" '3-LEG
SpreadList(7) = "BUTTERFLY4L" '4-LEG
SpreadList(8) = "IRON CONDOR"
SpreadList(9) = "~IRON CONDOR" 'sIRONCONDOR, special Iron Condor, or Iron condor w/ unequal # of call/put verticals
SpreadList(10) = "CUSTOM" 'DEVELOPMENT NEEDED
SpreadList(11) = "STOCK"
SpreadList(12) = "VERT ROLL"
SpreadList(13) = "UNDEFINED SPREAD"
Output_ASD.Add SpreadList
Output_ASD.Add SINGLEs_ASD
Output_ASD.Add VERTICALs_ASD
Output_ASD.Add CALENDARs_ASD
Output_ASD.Add DIAGONALs_ASD
Output_ASD.Add STRADDLEs_ASD
Output_ASD.Add STRANGLEs_ASD
Output_ASD.Add BUTTERFLY3s_ASD
Output_ASD.Add BUTTERFLY4s_ASD
Output_ASD.Add IRONCONDORs_ASD
Output_ASD.Add sIRONCONDORs_ASD
Output_ASD.Add CUSTOMs_ASD
Output_ASD.Add STOCKs_ASD
Output_ASD.Add VERTROLLs_ASD
Output_ASD.Add OTHERs_ASD
Set pAllTradesBySpread = Output_ASD
Set AllTradesBySpread = pAllTradesBySpread
End Property
Any feedback is appreciated.
Bookmarks