Modular declaration

tiredofit

Well-known Member
Joined
Apr 11, 2013
Messages
1,825
Office Version
  1. 365
  2. 2019
Platform
  1. Windows
This is in a class called Loan:

Code:
Option Explicit
  
    Dim mvPrincipalAmount As Variant
    Dim mvInterestRate As Variant
    Dim mvLoanNumber As Variant
    Dim mvTerm As Variant

Public Property Get PrincipalAmount() As Variant
    PrincipalAmount = mvPrincipalAmount
End Property

Public Property Let PrincipalAmount(ByVal vNewValue As Variant)
    mvPrincipalAmount = vNewValue
End Property

Public Property Get InterestRate() As Variant
    InterestRate = mvInterestRate
End Property

Public Property Let InterestRate(ByVal vNewValue As Variant)
    mvInterestRate = vNewValue
End Property

Public Property Get LoanNumber() As Variant
    LoanNumber = mvLoanNumber
End Property

Public Property Let LoanNumber(ByVal vNewValue As Variant)
    mvLoanNumber = vNewValue
End Property

Public Property Get Term() As Variant
    Term = mvTerm
End Property

Public Property Let Term(ByVal vNewValue As Variant)
    mvTerm = vNewValue
End Property

Public Property Get Payment() As Variant
    Payment = Application.WorksheetFunction.Pmt(mvInterestRate / 12, mvTerm, -mvPrincipalAmount)
End Property

This is in a standard module:

Code:
Option Explicit

Sub TestCollectionObject()

    Dim rg As Range
    Dim objLoans As Collection
    Dim objLoan As Loan

    Set rg = ThisWorkbook.Worksheets("Loans").Range("LoanListStart").Offset(1, 0)

    ' get the collection of loan objects

    Set objLoans = CollectLoanObjects(rg)

    Debug.Print "There are " & objLoans.Count & " loans."

    ' iterate through each loan

    For Each objLoan In objLoans

        Debug.Print "Loan Number " & objLoan.LoanNumber & " has a payment of " & Format(objLoan.Payment, "Currency")

    Next

    Set objLoans = Nothing
    Set objLoan = Nothing
    Set rg = Nothing

End Sub

Function CollectLoanObjects(rg As Range) As Collection

    Dim objLoan As Loan
    Dim objLoans As Collection
    Set objLoans = New Collection

    ' loop until we find an empty row

    Do Until IsEmpty(rg)

        Set objLoan = New Loan

        With objLoan

            .LoanNumber = rg.Value
            .Term = rg.Offset(0, 1).Value
            .InterestRate = rg.Offset(0, 2).Value
            .PrincipalAmount = rg.Offset(0, 3).Value

        End With

        ' add the current loan to the collection

        objLoans.Add objLoan, CStr(objLoan.LoanNumber)

        ' move to next row

        Set rg = rg.Offset(1, 0)

    Loop

    Set objLoan = Nothing
    Set CollectLoanObjects = objLoans
    Set objLoans = Nothing

End Function

The code works fine but I noticed both objLoan and objLoans have been declared twice, once in TestCollectionObject and once in the function.

Would it be possible, possibly even preferable, to declare it only once at the modular level, ie:

Code:
Dim objLoan As Loan
Dim objLoans As Collection

Sub TestCollectionObject()

    ' etc.

or have I missed the point of why they have been declared separately?

Thanks
 

Excel Facts

Format cells as time
Select range and press Ctrl+Shift+2 to format cells as time. (Shift 2 is the @ sign).
I wasn't even aware that it was possible to declare the same variable twice at different levels until it was pointed out to me as a potential flaw in something that I suggested in another thread recently. I've just been looking for the thread so that I could link to it here, but haven't been able to find it, either I didn't go back far enough or I missed it due to a poor thread title.

I can only see the variables that you mention declared and used at procedure level.

preferable, to declare it only once
Based on what I remember of the discussion in the earlier thread, it would be preferable to declare it once only, in the Sub, not at modular level.
Modular level declarations are only needed if the variable is being passed between multiple procedures, from the code that you have posted I don't see that happening.
In the event that the modular level variable is being used in another procedure, the Sub level variable would have priority anyway.

Having it declared as modular but not sub level could result in 'dirty' variables being passed to the procedure, although your code should prevent this being a problem.
Declaring at Sub level means that you don't need to set variables to Nothing before you finish, it is automatic when the code hits End Sub.
 
Upvote 0
I wasn't even aware that it was possible to declare the same variable twice at different levels until it was pointed out to me as a potential flaw in something that I suggested in another thread recently. I've just been looking for the thread so that I could link to it here, but haven't been able to find it, either I didn't go back far enough or I missed it due to a poor thread title.

I can only see the variables that you mention declared and used at procedure level.


Based on what I remember of the discussion in the earlier thread, it would be preferable to declare it once only, in the Sub, not at modular level.
Modular level declarations are only needed if the variable is being passed between multiple procedures, from the code that you have posted I don't see that happening.
In the event that the modular level variable is being used in another procedure, the Sub level variable would have priority anyway.

Having it declared as modular but not sub level could result in 'dirty' variables being passed to the procedure, although your code should prevent this being a problem.
Declaring at Sub level means that you don't need to set variables to Nothing before you finish, it is automatic when the code hits End Sub.

Thanks for your reply.

I thought it was more efficient to be declaring it only once at the modular level.

I am still "stuck" in applying mathematical principles to programming, eg ab+ac = a(b+c), so here I thought instead of declaring the variables in two separate places, just do it once!
 
Upvote 0
Hang on, let me go through the code again. For some reason, the second block of code in your post didn't scroll all the way down, I missed the Function at the bottom.
 
Upvote 0
Without having suitable data to test the code, I can't say for certain, what problems you may or may not encounter by changing anything.

Personally, I would keep it as it is, but change the names in either the Sub or the Function so that they are unique.

Declaring the existing names at module level and removing them from the individual procedures could result in problems if control is passed with an incorrect value assigned to either of them.
 
Upvote 0
Without having suitable data to test the code, I can't say for certain, what problems you may or may not encounter by changing anything.

Personally, I would keep it as it is, but change the names in either the Sub or the Function so that they are unique.

Declaring the existing names at module level and removing them from the individual procedures could result in problems if control is passed with an incorrect value assigned to either of them.
Thanks

I'll post some data later.
 
Upvote 0
Or you could just test it yourself.

Create 2 identical test files, run the original code on one, then modify the code and run it on the second copy, compare the results in the 2 files for any anomalies.
 
Upvote 0
Or you could just test it yourself.

Create 2 identical test files, run the original code on one, then modify the code and run it on the second copy, compare the results in the 2 files for any anomalies.

I've tested it, declaring it once only as well as twice and I got identical results.

FYI, the data is as follows:

Note the named range in cell A1 and the sheetname is Loans.



1591608235461.png


On a separate note, I have a question re these lines:

Code:
Dim objLoans As Collection
Dim objLoan As Loan
Set rg = ThisWorkbook.Worksheets("Loans").Range("LoanListStart").Offset(1, 0)

' get the collection of loan objects

Set objLoans = CollectLoanObjects(rg)

whenever I see this:

Code:
Dim objLoans As Collection

I expect to see this later:

Code:
Set objLoans = New Collection

but here instead, it's:

Code:
Set objLoans = CollectLoanObjects(rg)

I find this concept hard to grasp.

I can only understand because it's added the comment that it's getting the collection of loan objects.
 
Last edited:
Upvote 0
You should leave the code as it is.
 
Upvote 0
Not sure if there is something that I'm missing, but in my opinion the code appears to be over-engineered enough to work either way, my thought was something more like the quick edit below, although I am wondering if the function is even necessary, or if the entire collection could be populated directly in the sub as an array without looping?
VBA Code:
Option Explicit

Sub TestCollectionObject()

    Dim rg As Range
    Dim objLoans As Collection
    Dim objLoan As Loan

    Set rg = ThisWorkbook.Worksheets("Loans").Range("LoanListStart").Offset(1, 0)

    ' get the collection of loan objects

    Set objLoans = CollectLoanObjects(rg)

    Debug.Print "There are " & objLoans.Count & " loans."

    ' iterate through each loan

    For Each objLoan In objLoans

        Debug.Print "Loan Number " & objLoan.LoanNumber & " has a payment of " & Format(objLoan.Payment, "Currency")

    Next

End Sub

Function CollectLoanObjects(rg As Range) As Collection

    Dim FobjLoan As Loan
    Dim FobjLoans As Collection
    Set FobjLoans = New Collection

    ' loop until we find an empty row

    Do Until IsEmpty(rg)

        Set FobjLoan = New Loan

        With FobjLoan

            .LoanNumber = rg.Value
            .Term = rg.Offset(0, 1).Value
            .InterestRate = rg.Offset(0, 2).Value
            .PrincipalAmount = rg.Offset(0, 3).Value

        End With

        ' add the current loan to the collection

        FobjLoans.Add FobjLoan, CStr(FobjLoan.LoanNumber)

        ' move to next row

        Set rg = rg.Offset(1, 0)

    Loop

    Set CollectLoanObjects = FobjLoans

End Function
 
Upvote 0

Forum statistics

Threads
1,213,562
Messages
6,114,326
Members
448,564
Latest member
ED38

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top