Using property instead of passing arguments

[tiredofit]

When learning about classes, one is usually told that instead of declaring variables as Public, we should use Get, Let and Set Properties, which I agree.

As a result, I see no reason why you should write the following within a class:

Sub SomeMethod (SomeArg As SomeDataType)

    ' do something

End Sub

because instead of passing SomeArg via the method, its value would be determined using the Let Property beforehand.

Furthermore, I think this would extend to standard modules, ie one could (and probably should) use Properties in a standard module too and do away with passing arguments to Subs.

What is the flaw with my reasoning, other than it takes a bit more work to set up the Properties?

Thanks

 

[6StringJazzer]

When designing a class, you must decide what data constitutes the state of the class. This data should be declared as Private variables, and touched only by Get, Let, and Set methods. (You will not need all of these for each state data variable, by the way.)

When providing a method, if that method operates on the state data, that data should be accessed internally to the object and not passed in as an argument. However, in your example, you have no context for what your method does or what SomeArg is. It is perfectly valid, and quite common, to have methods with arguments. Your question would be much easier to understand with a concrete example.

Standard modules can be treated as classes, but in the end they are not actually classes since you cannot instantiate them. You can think of it as a static class, but I hesitate to draw too many parallels to OOP here since VBA is not a pure OOL.

It would probably be a good practice for a standard module to have private variables and if those variables are equivalent to class state data. But that does not eliminate all need to pass arguments to Subs. So I really don't know how you envision this working.

Again, this would be much clearer to discuss with a concrete example.

 

[tiredofit]

When designing a class, you must decide what data constitutes the state of the class. This data should be declared as Private variables, and touched only by Get, Let, and Set methods. (You will not need all of these for each state data variable, by the way.)

When providing a method, if that method operates on the state data, that data should be accessed internally to the object and not passed in as an argument. However, in your example, you have no context for what your method does or what SomeArg is. It is perfectly valid, and quite common, to have methods with arguments. Your question would be much easier to understand with a concrete example.

Standard modules can be treated as classes, but in the end they are not actually classes since you cannot instantiate them. You can think of it as a static class, but I hesitate to draw too many parallels to OOP here since VBA is not a pure OOL.

It would probably be a good practice for a standard module to have private variables and if those variables are equivalent to class state data. But that does not eliminate all need to pass arguments to Subs. So I really don't know how you envision this working.

Again, this would be much clearer to discuss with a concrete example.

Thanks for your reply.

Here is an example, illustrating my point in standard modules:

Method 1.

'Module1

Option Explicit

Sub Start()

    Dim ws As Worksheet
    Set ws = ActiveSheet
    
    Call Somesub(ws)
    
    Set ws = Nothing
    
End Sub

Sub Somesub(ByRef ws As Worksheet)

    Select Case ws.CodeName
    
        Case Sheet1.CodeName
        
            MsgBox "Sheet1"
            
        Case Sheet2.CodeName
    
            MsgBox "Sheet2"
            
        Case Else
        
            MsgBox "Not Sheet1 nor Sheet2"
            
    End Select

End Sub

This is Method 2.

' Module2

Option Explicit

    Dim pSheet As Worksheet
    
Public Property Get Sheet() As Worksheet

    Set Sheet = pSheet
    
End Property

Public Property Set Sheet(ByVal S As Worksheet)

    Set pSheet = S
    
End Property

Sub Start()

    Set Module2.Sheet = ActiveSheet
    
    Call Somesub
    
End Sub

Sub Somesub()

    Select Case Sheet.CodeName
    
        Case Sheet1.CodeName
        
            MsgBox "Sheet1"
            
        Case Sheet2.CodeName
    
            MsgBox "Sheet2"
            
        Case Else
        
            MsgBox "Not Sheet1 nor Sheet2"
            
    End Select

End Sub

 

[Kyle123]

I was writing pretty much the same @6StringJazzer, but he's done a better job so I deleted mine .

I would add, that I do use modules as quasi static singletons for read only "globals", I find it neat solution as it allows for the setting of variables on their first access without them being editable

 

[Kyle123]

On your example, look how much easier it is to read and follow the first example. Think of properties only for storing state, the less state the better - it easier to test methods that don't have state and/or side effects

 

[tiredofit]

On your example, look how much easier it is to read and follow the first example. Think of properties only for storing state, the less state the better - it easier to test methods that don't have state and/or side effects

Perhaps you have already answered my query above but I'm posting the following code to illustrate my point, this time, it includes class modules.

Standard module:

Option Explicit

Public Sub StartKnightsTour()

On Error GoTo ERR_HANDLER:

    Dim rngSquare       As Range
    Dim rngSquareNext   As Range
    Dim oKnight         As New clsKnightTour
    
    oKnight.BoardArea = Range("Board")
    Set rngSquare = oKnight.GetRandomSquareFromBoard
    oKnight.PieceAscCode = 140
    oKnight.PieceFontName = "Chess Alpha"
    oKnight.DisplayPiece rngSquare
    With oKnight
        Do
            Set rngSquareNext = .GetNextMove(rngSquare)
            If rngSquareNext Is Nothing Then
                Exit Do 'no more possible moves
            Else
                .MovePiece rngSquare, rngSquareNext
            End If
            Set rngSquare = rngSquareNext
        Loop
    End With
EXIT_HERE:
    Set rngSquare = Nothing
    Set rngSquareNext = Nothing
    Set oKnight = Nothing
    Exit Sub
ERR_HANDLER:
    Debug.Print Err.Description
    GoTo EXIT_HERE
End Sub

clsFontInstall

Option Explicit

Private Declare Function AddFontResource Lib "gdi32" Alias "AddFontResourceA" (ByVal lpFileName As String) As Long
Private Declare Function RemoveFontResource Lib "gdi32" Alias "RemoveFontResourceA" (ByVal lpFileName As String) As Long

Private Declare Function SendMessageTimeoutA Lib "user32.dll" ( _
  ByVal hWnd As Long, _
  ByVal Msg As Long, _
  ByVal wParam As Long, _
  ByRef lParam As Any, _
  ByVal fuFlags As Long, _
  ByVal uTimeout As Long, _
  ByRef lpdwResult As Long) As Long

Private Const WM_FONTCHANGE         As Long = &H1D
Private Const HWND_BROADCAST        As Long = &HFFFF&
Private Const SMTO_NORMAL           As Long = &H0
Private Const cintMatch             As Integer = 0

Private mstrFontName                As String
Private mstrFontFileName            As String
Private mlngMilliSeconds            As Long

Public Property Get FontName() As String
    FontName = mstrFontName
End Property

Public Property Let FontName(ByVal Value As String)
    mstrFontName = Value
End Property

Public Property Let FontFileName(ByVal Value As String)
     mstrFontFileName = Value
End Property

Public Property Let NotifyWindowsTimeOut(ByVal Value As Integer)
     mlngMilliSeconds = Value
End Property

Public Function UninstallFonts() As Boolean
On Error GoTo errUninstallHandler
    UninstallFonts = False
    If RemoveFontResource(mstrFontFileName) Then
        Dim lngReturn   As Long
        Dim lResult     As Long

        lngReturn = SendMessageTimeoutA( _
            HWND_BROADCAST, _
            WM_FONTCHANGE, _
            0, _
            ByVal "windows", _
            SMTO_NORMAL, _
            1000, _
            lResult)
        UninstallFonts = True
    Else
        UninstallFonts = False
    End If
EXIT_HERE:
    Exit Function
errUninstallHandler:
    UninstallFonts = False
    GoTo EXIT_HERE
End Function


Public Function InstallFonts() As Boolean
On Error GoTo errInstallHandler
    Dim lngReturn As Long
    Dim lngResult As Long
    InstallFonts = True

    If IsFontInstalled = False Then
        If AddFontResource(mstrFontFileName) Then
            lngReturn = SendMessageTimeoutA( _
                HWND_BROADCAST, _
                WM_FONTCHANGE, _
                0, _
                ByVal "windows", _
                SMTO_NORMAL, _
                mlngMilliSeconds, _
                lngResult)
        Else
            InstallFonts = False
        End If
    End If
EXIT_HERE:
    Exit Function
errInstallHandler:
    InstallFonts = False
    GoTo EXIT_HERE
End Function

Public Function IsFontInstalled() As Boolean
On Error GoTo ErrHandler:
    Dim objFont As New StdFont
    IsFontInstalled = False
    
    objFont.Name = mstrFontName
    If StrComp(mstrFontName, objFont.Name, vbTextCompare) = cintMatch Then
        IsFontInstalled = True
    Else
        IsFontInstalled = False
    End If
ExitHere:
    Set objFont = Nothing
    Exit Function
ErrHandler:
    IsFontInstalled = False
    GoTo ExitHere
End Function

Private Sub Class_Initialize()
    mlngMilliSeconds = 1000
End Sub

Private Sub Class_Terminate()
    On Error Resume Next
    If IsFontInstalled = True Then
        Me.UninstallFonts
    End If
End Sub

clsKnightTour

Option Explicit

#If VBA7 Then
    Private Declare PtrSafe Sub Sleep Lib "kernel32" (ByVal dwMilliseconds As LongPtr) 'For 64 Bit Systems
#Else
    Private Declare Sub Sleep Lib "kernel32" (ByVal dwMilliseconds As Long)            'For 32 Bit Systems
#End If

Private Type udtLocation
    RowValue    As Long
    ColumnValue As Integer
End Type

Private mudtPosition(1 To 8)    As udtLocation
Private mrngArrVisited()        As Range
Private mrngBoard               As Range
Private mintVisitedCnt          As Integer
Private mintSequence            As Integer
Private mintPieceAscCode        As Integer
Private mlngDelay               As Long
Private mstrPieceFontName       As String

Private Const CINT_VISITED_COLORINDEX   As Integer = 1
Private Const CINT_CURRENT_COLORINDEX   As Integer = 45
Private Const CINT_CURRENT_SYMBOL_SIZE  As Integer = 28
Private Const CINT_VISITED_SYMBOL_SIZE  As Integer = 8
Private Const CINT_DEFAULT_ASCII_CHAR   As Integer = 88
Private Const CLNG_DEFAULT_DELAY        As Long = 250
Private Const CSTR_DEFAULT_FONT         As String = "Arial"

Private Sub Class_Initialize()
    mlngDelay = CLNG_DEFAULT_DELAY
    mudtPosition(1).RowValue = -2: mudtPosition(1).ColumnValue = 1
    mudtPosition(2).RowValue = 1: mudtPosition(2).ColumnValue = 2
    mudtPosition(3).RowValue = 2: mudtPosition(3).ColumnValue = 1
    mudtPosition(4).RowValue = -1: mudtPosition(4).ColumnValue = 2
    mudtPosition(5).RowValue = -2: mudtPosition(5).ColumnValue = -1
    mudtPosition(6).RowValue = -1: mudtPosition(6).ColumnValue = -2
    mudtPosition(7).RowValue = 1: mudtPosition(7).ColumnValue = -2
    mudtPosition(8).RowValue = 2: mudtPosition(8).ColumnValue = -1
    mintSequence = 1
    ReDim mrngArrVisited(0 To 0)
    mintVisitedCnt = 0
    mstrPieceFontName = CSTR_DEFAULT_FONT
    mintPieceAscCode = CINT_DEFAULT_ASCII_CHAR
End Sub

Public Property Let PieceFontName(ByVal Value As String)
    mstrPieceFontName = Value
End Property

Public Property Let PieceAscCode(ByVal Value As Integer)
    mintPieceAscCode = Value
End Property

Public Property Let BoardArea(ByVal Value As Range)
    Set mrngBoard = Value
    mrngBoard.ClearContents
End Property

Private Function IsValidPosition(ByVal rngCell As Range) As Boolean
    If Intersect(rngCell, mrngBoard) Is Nothing Then
        IsValidPosition = False
    Else
        IsValidPosition = True
    End If
End Function

Public Function GetRandomSquareFromBoard() As Range
    Dim intRndCell          As Integer
    Dim intBoardCellsCnt    As Integer

    If Not mrngBoard Is Nothing Then
        intBoardCellsCnt = mrngBoard.Cells.Count
        intRndCell = GetRandomNumber(intBoardCellsCnt, 1)
        Set GetRandomSquareFromBoard = mrngBoard.Cells(intRndCell)
    End If
End Function

Public Sub RemovePiece(ByVal rngSquare As Range)
    With rngSquare
        .Font.Size = CINT_VISITED_SYMBOL_SIZE
        .Font.Bold = True
        .Font.ColorIndex = CINT_VISITED_COLORINDEX
        .Font.Name = CSTR_DEFAULT_FONT
    End With
End Sub

Public Sub DisplayPiece(ByVal rngSquare As Range)
    With rngSquare
        .Font.Size = CINT_CURRENT_SYMBOL_SIZE
        .Font.Bold = True
        .Font.ColorIndex = CINT_CURRENT_COLORINDEX
        .Value = Chr(mintPieceAscCode)
        .Font.Name = mstrPieceFontName
    End With
End Sub

Public Sub MovePiece(ByVal rngFrom As Range, ByVal rngTo As Range)
    Application.ScreenUpdating = False
    Sleep mlngDelay
    RemovePiece rngFrom
    rngFrom.Value = mintSequence
    mintSequence = mintSequence + 1
    DisplayPiece rngTo
    Application.ScreenUpdating = True
End Sub

Public Function GetNextMove(ByVal rngCell As Range) As Range
    Dim intCnt                      As Integer
    Dim intMaxMoves                 As Integer
    Dim intMoves                    As Integer
    Dim intArrCnt                   As Integer
    Dim intRnd                      As Integer
    Dim rngNewLocation              As Range
    Dim arrListOfSquaresToMoveTo()  As Range
    
    intArrCnt = 0
    intMoves = 0
    intMaxMoves = UBound(mudtPosition)
    
    ReDim Preserve mrngArrVisited(0 To mintVisitedCnt)
    Set mrngArrVisited(mintVisitedCnt) = rngCell
    mintVisitedCnt = mintVisitedCnt + 1

    For intCnt = LBound(mudtPosition) To UBound(mudtPosition)
        DoEvents
        Set rngNewLocation = rngCell.Offset(mudtPosition(intCnt).RowValue, mudtPosition(intCnt).ColumnValue)
        If IsValidPosition(rngNewLocation) Then
            If Not IsVisitedLocation(rngNewLocation) Then
                intMoves = CountPossibleMovesFromLocation(rngNewLocation)
                Select Case intMoves
                Case Is < intMaxMoves
                     intMaxMoves = intMoves
                     intArrCnt = 0
                     ReDim arrListOfSquaresToMoveTo(0 To intArrCnt)
                     Set arrListOfSquaresToMoveTo(intArrCnt) = rngNewLocation
                     intArrCnt = intArrCnt + 1
                Case intMaxMoves
                     ReDim Preserve arrListOfSquaresToMoveTo(0 To intArrCnt)
                     Set arrListOfSquaresToMoveTo(intArrCnt) = rngNewLocation
                     intArrCnt = intArrCnt + 1
                End Select
            End If
        End If
    Next

    If intArrCnt > 0 Then
        intRnd = GetRandomNumber(UBound(arrListOfSquaresToMoveTo), LBound(arrListOfSquaresToMoveTo))
        Set GetNextMove = arrListOfSquaresToMoveTo(intRnd)
    End If
    
    Set rngNewLocation = Nothing
End Function

Private Function CountPossibleMovesFromLocation(ByVal rngCell As Range) As Integer
    Dim intPos          As Integer
    Dim intCnt          As Integer
    Dim rngNewLocation  As Range
    
    intCnt = 0
    For intPos = LBound(mudtPosition) To UBound(mudtPosition)
        DoEvents
        Set rngNewLocation = rngCell.Offset(mudtPosition(intPos).RowValue, mudtPosition(intPos).ColumnValue)
        If IsValidPosition(rngNewLocation) Then
            If Not IsVisitedLocation(rngNewLocation) Then
                intCnt = intCnt + 1
            End If
        End If
    Next
    Set rngNewLocation = Nothing
    CountPossibleMovesFromLocation = intCnt
End Function

Private Function IsVisitedLocation(ByVal rngCell As Range) As Boolean
    Dim intCnt As Integer

    IsVisitedLocation = False
    If mintVisitedCnt = 0 Then
        Exit Function
    End If
    
    For intCnt = LBound(mrngArrVisited) To UBound(mrngArrVisited)
        DoEvents
        If mrngArrVisited(intCnt).Address = rngCell.Address Then
            IsVisitedLocation = True
            Exit Function
        End If
    Next
End Function

Private Function GetRandomNumber(ByVal lngMaxValue As Long, Optional ByVal lngMinValue As Long = 0)
    Randomize
    GetRandomNumber = Int((lngMaxValue - lngMinValue + 1) * Rnd) + lngMinValue
End Function

In the standard module, this line:

.MovePiece rngSquare, rngSquareNext

calls the method oKnight.MovePiece, passing in two parameters.

My point is since clsKnightTour already contains some properties, why weren't two more added, like:

Private prng As Range
Private prngNext As Range

Public Property Get rng() As Range
    
    Set rng = prng
    
End Property

Public Property Set rng(ByVal r As Range)

    Set prng = r
    
End Property

Public Property Get rngNext() As Range

    Set rngNext = prngNext
    
End Property

Public Property Set rngNext(ByVal rNext As Range)

    Set prngNext = rNext
    
End Property

and change the method to:

Public Sub MovePiece()
    Application.ScreenUpdating = False
    Sleep mlngDelay
    RemovePiece Me.rng
    Me.rng.Value = mintSequence
    mintSequence = mintSequence + 1
    DisplayPiece Me.rngNext
    Application.ScreenUpdating = True
End Sub

then in the standard module, write:

With oKnight
        Do
            Set rngSquareNext = .GetNextMove(rngSquare)
            If rngSquareNext Is Nothing Then
                Exit Do 'no more possible moves
            Else
                Set .rng = rngSquare '**********************ADDED THIS
                Set .rngNext = rngSquareNext '**********************ADDED THIS
                
                Call .MovePiece '**************************CHANGED HERE

            End If
            Set rngSquare = rngSquareNext
        Loop
    End With

 

[6StringJazzer]

In your module example, the module does not act like an object, it performs a service. It is performing a service on a sheet, and the caller provides the sheet. In my opinion the sheet is not an attribute of the module, so should not be modeled that way.

 

[6StringJazzer]

I am behind the thread. I will read your most recent code and get back a little later.