trjhtr

The challenge I'm facing is creating a class for a Loan object which contains a number of properties, some of which are classes in and of themselves with similar attributes (e.g., Lender) as well as some List(Of...) properties (Borrowers and collateral). I don't want to have these classes load their properties unless specifically called for a couple of reasons:

• Prevent unnecessary calls to the database


• Try to prevent the use of "stale" data in the parent class when referring to these properties


• When it's a List(Of...) property, things can get extra dicey

In order to accomplish this, I've put together the following structure, which appears to be working normally. However, I'm really beginning to wonder if I'm overthinking this and making it much more complicated than it needs to be - mostly in the List(Of...) properties. Any suggestions are most definitely welcome.

#Region "LOAN RECORD"
 ''' <summary>
 ''' Standard object containing details about a specific loan record
 ''' </summary>
 Public Class Loan
#Region "PRIVATE PROPERTIES"
 <EditorBrowsable(EditorBrowsableState.Never)> <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Private LenderID As Integer

 <EditorBrowsable(EditorBrowsableState.Never)> <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Private Property BorrowerIDs As List(Of Integer)

 <EditorBrowsable(EditorBrowsableState.Never)> <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Private Property VehicleIDs As List(Of Integer)

 <EditorBrowsable(EditorBrowsableState.Never)> <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Private Property RealEstatePropertyIDs As List(Of Integer)

#Region "BUFFERS TO PREVENT UNNECESSARY DATABASE CALLS"
 <EditorBrowsable(EditorBrowsableState.Never)> <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Private _Lender As Lender

 <EditorBrowsable(EditorBrowsableState.Never)> <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Private _Borrowers As List(Of Borrower)

 <EditorBrowsable(EditorBrowsableState.Never)> <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Private _Vehicles As List(Of VehicleCollateral)

 <EditorBrowsable(EditorBrowsableState.Never)> <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Private _RealEstateProperties As List(Of RealEstateCollateral)
#End Region
#End Region

#Region "PUBLIC PROPERTIES"
 Public Property LoanID As Integer
 Public Property LoanNumber As String
 Public Property InceptionDate As Nullable(Of Date)
 Public Property MaturityDate As Nullable(Of Date)
 Public Property CurrentBalance As Decimal
 Public Property CreditLimit As Decimal
 Public Property InterestRate As Decimal
 Public Property PaymentFrequency As Integer
 Public Property CurrentPaymentAmount As Decimal
 Public Property PaidDate As Nullable(Of Date)

 <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Public Property Lender As Lender
 Get
 If Me._Lender Is Nothing OrElse Me._Lender.LenderID <> Me.LenderID Then
 Me._Lender = New Lender(Me.LenderID)
 End If

 Return Me._Lender
 End Get

 Set(value As Lender)
 Me.LenderID = value.LenderID
 Me._Lender = value
 End Set
 End Property

 <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Public Property Borrowers As List(Of Borrower)
 Get
 If Not Me.BorrowerIDs Is Nothing AndAlso Me.BorrowerIDs.Count > 0 Then
 If Me._Borrowers Is Nothing OrElse Me._Borrowers.Count <> Me.BorrowerIDs.Count Then
 Dim LoanBorrowers As New List(Of Borrower)

 For Each BorrowerID As Integer In Me.BorrowerIDs
 If BorrowerID > 0 Then
 LoanBorrowers.Add(New Borrower(BorrowerID))
 End If
 Next BorrowerID

 If LoanBorrowers.Count > 0 Then
 If Not Me._Borrowers Is Nothing Then
 Me._Borrowers.Clear()
 Else
 Me._Borrowers = New List(Of Borrower)
 Me._Borrowers.Clear()
 End If

 Me._Borrowers = LoanBorrowers
 Else
 Me._Borrowers = Nothing
 End If
 Else
 Dim Reload As Boolean = False

 For Each LoanBorrower As Borrower In Me._Borrowers
 If Not Me.BorrowerIDs.Contains(LoanBorrower.BorrowerID) Then
 Reload = True
 Exit For
 End If
 Next LoanBorrower

 If Reload Then
 Dim LoanBorrowers As New List(Of Borrower)

 For Each BorrowerID As Integer In Me.BorrowerIDs
 If BorrowerID > 0 Then
 LoanBorrowers.Add(New Borrower(BorrowerID))
 End If
 Next BorrowerID

 If LoanBorrowers.Count > 0 Then
 If Not Me._Borrowers Is Nothing Then
 Me._Borrowers.Clear()
 Else
 Me._Borrowers = New List(Of Borrower)
 Me._Borrowers.Clear()
 End If

 Me._Borrowers = LoanBorrowers
 Else
 Me._Borrowers = Nothing
 End If
 End If
 End If
 Else
 Me._Borrowers = Nothing
 End If

 Return Me._Borrowers
 End Get

 Set(value As List(Of Borrower))
 If Not value Is Nothing AndAlso value.Count > 0 Then
 If Me.BorrowerIDs Is Nothing Then
 Me.BorrowerIDs = New List(Of Integer)
 End If

 Me.BorrowerIDs.Clear()

 For Each valueBorrower As Borrower In value
 Me.BorrowerIDs.Add(valueBorrower.BorrowerID)
 Next valueBorrower

 Me._Borrowers = value
 Else
 Me._Borrowers = Nothing
 End If
 End Set
 End Property

 <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Public Property Vehicles As List(Of VehicleCollateral)
 Get
 If Not Me.VehicleIDs Is Nothing AndAlso Me.VehicleIDs.Count > 0 Then
 If Me._Vehicles Is Nothing OrElse Me._Vehicles.Count <> Me.VehicleIDs.Count Then
 Dim Collateral As New List(Of VehicleCollateral)

 For Each CollateralID As Integer In Me.VehicleIDs
 If CollateralID > 0 Then
 Collateral.Add(New VehicleCollateral(CollateralID))
 End If
 Next CollateralID

 If Collateral.Count > 0 Then
 If Not Me._Vehicles Is Nothing Then
 Me._Vehicles.Clear()
 Else
 Me._Vehicles = New List(Of VehicleCollateral)
 Me._Vehicles.Clear()
 End If

 Me._Vehicles = Collateral
 Else
 Me._Vehicles = Nothing
 End If
 Else
 Dim Reload As Boolean = False

 For Each Vehicle As VehicleCollateral In Me._Vehicles
 If Not Me.VehicleIDs.Contains(Vehicle.CollateralID) Then
 Reload = True
 Exit For
 End If
 Next Vehicle

 If Reload Then
 Dim Collateral As New List(Of VehicleCollateral)

 For Each CollateralID As Integer In Me.VehicleIDs
 If CollateralID > 0 Then
 Collateral.Add(New VehicleCollateral(CollateralID))
 End If
 Next CollateralID

 If Collateral.Count > 0 Then
 If Not Me._Vehicles Is Nothing Then
 Me._Vehicles.Clear()
 Else
 Me._Vehicles = New List(Of VehicleCollateral)
 Me._Vehicles.Clear()
 End If

 Me._Vehicles = Collateral
 Else
 Me._Vehicles = Nothing
 End If
 End If
 End If
 Else
 Me._Vehicles = Nothing
 End If

 Return Me._Vehicles
 End Get

 Set(value As List(Of VehicleCollateral))
 If Not value Is Nothing AndAlso value.Count > 0 Then
 If Me.VehicleIDs Is Nothing Then
 Me.VehicleIDs = New List(Of Integer)
 End If

 Me.VehicleIDs.Clear()

 For Each valueCollateral As VehicleCollateral In value
 Me.VehicleIDs.Add(valueCollateral.CollateralID)
 Next valueCollateral

 Me._Vehicles = value
 Else
 Me._Vehicles = Nothing
 End If
 End Set
 End Property

 <DebuggerBrowsable(DebuggerBrowsableState.Never)>
 Public Property RealEstateProperties As List(Of RealEstateCollateral)
 Get
 If Not Me.RealEstatePropertyIDs Is Nothing AndAlso Me.RealEstatePropertyIDs.Count > 0 Then
 If Me._RealEstateProperties Is Nothing OrElse Me._RealEstateProperties.Count <> Me.RealEstatePropertyIDs.Count Then
 Dim Collateral As New List(Of RealEstateCollateral)

 For Each CollateralID As Integer In Me.RealEstatePropertyIDs
 If CollateralID > 0 Then
 Collateral.Add(New RealEstateCollateral(CollateralID))
 End If
 Next CollateralID

 If Collateral.Count > 0 Then
 If Not Me._RealEstateProperties Is Nothing Then
 Me._RealEstateProperties.Clear()
 Else
 Me._RealEstateProperties = New List(Of RealEstateCollateral)
 Me._RealEstateProperties.Clear()
 End If

 Me._RealEstateProperties = Collateral
 Else
 Me._RealEstateProperties = Nothing
 End If
 Else
 Dim Reload As Boolean = False

 For Each REProperty As RealEstateCollateral In Me._RealEstateProperties
 If Not Me.RealEstatePropertyIDs.Contains(REProperty.CollateralID) Then
 Reload = True
 Exit For
 End If
 Next REProperty

 If Reload Then
 Dim Collateral As New List(Of RealEstateCollateral)

 For Each CollateralID As Integer In Me.RealEstatePropertyIDs
 If CollateralID > 0 Then
 Collateral.Add(New RealEstateCollateral(CollateralID))
 End If
 Next CollateralID

 If Collateral.Count > 0 Then
 If Not Me._RealEstateProperties Is Nothing Then
 Me._RealEstateProperties.Clear()
 Else
 Me._RealEstateProperties = New List(Of RealEstateCollateral)
 Me._RealEstateProperties.Clear()
 End If

 Me._RealEstateProperties = Collateral
 Else
 Me._RealEstateProperties = Nothing
 End If
 End If
 End If
 Else
 Me._RealEstateProperties = Nothing
 End If

 Return Me._RealEstateProperties
 End Get

 Set(value As List(Of RealEstateCollateral))
 If Not value Is Nothing AndAlso value.Count > 0 Then
 If Me.RealEstatePropertyIDs Is Nothing Then
 Me.RealEstatePropertyIDs = New List(Of Integer)
 End If

 Me.RealEstatePropertyIDs.Clear()

 For Each valueCollateral As RealEstateCollateral In value
 Me.RealEstatePropertyIDs.Add(valueCollateral.CollateralID)
 Next valueCollateral

 Me._RealEstateProperties = value
 Else
 Me._RealEstateProperties = Nothing
 End If
 End Set
 End Property
#End Region

#Region "PUBLIC METHODS"
#Region "CONSTRUCTORS"
 ''' <summary>
 ''' Creates a new instance of a Loan object with default values
 ''' </summary>
 Public Sub New()
 Me.Initialize()
 End Sub

 ''' <summary>
 ''' Creates a new instance of a Loan object based on the internal ID assigned to the database record
 ''' </summary>
 ''' <param name="NewLoanID">Database-assigned identification number for a specific Loan record</param>
 Public Sub New(ByVal NewLoanID As Integer)
 Me.Initialize()
 Me.GetLoanDetail(NewLoanID)
 End Sub
#End Region

 Public Shared Function FindExistingLoan(ByVal LoanNumber As String, ByVal LenderID As Integer) As LoanDetail.Loan
 Dim FoundLoan As New LoanDetail.Loan

 FoundLoan.GetLoanDetail(LoanNumber, LenderID)

 Return FoundLoan
 End Function
#End Region

#Region "PRIVATE METHODS"
 ''' <summary>
 ''' Sets all of the values to default for a new instance of a Loan object
 ''' </summary>
 Private Sub Initialize()
 Me.LoanID = 0
 Me.LenderID = 0
 Me.LoanNumber = String.Empty
 Me.InceptionDate = Nothing
 Me.MaturityDate = Nothing
 Me.CurrentBalance = 0
 Me.CreditLimit = 0
 Me.InterestRate = 0
 Me.PaymentFrequency = 12
 Me.CurrentPaymentAmount = 0
 Me.PaidDate = Nothing

 Me.BorrowerIDs = New List(Of Integer)
 Me.VehicleIDs = New List(Of Integer)
 Me.RealEstatePropertyIDs = New List(Of Integer)

 Me._Lender = Nothing
 Me._Borrowers = Nothing
 Me._Vehicles = Nothing
 Me._RealEstateProperties = Nothing
 End Sub

 ''' <summary>
 ''' Populates all of the available values for the Loan object from the database record
 ''' </summary>
 ''' <param name="DBData">DataRow containing the details for the specified Loan record from the database</param>
 Private Sub Fill(ByRef DBData As DataRow)
 With DBData
 Me.LoanID = Convert.ToInt32(.Item("LoanID"))
 Me.LenderID = Convert.ToInt32(.Item("LenderID"))
 Me.LoanNumber = Convert.ToString(.Item("LoanNumber"))

 If Not IsDBNull(.Item("InceptionDate")) Then
 Me.InceptionDate = Convert.ToDateTime(.Item("InceptionDate"))
 End If

 If Not IsDBNull(.Item("MaturityDate")) Then
 Me.MaturityDate = Convert.ToDateTime(.Item("MaturityDate"))
 End If

 If Not IsDBNull(.Item("CurrentBalance")) Then
 Me.CurrentBalance = Convert.ToDecimal(.Item("CurrentBalance"))
 End If

 If Not IsDBNull(.Item("CreditLimit")) Then
 Me.CreditLimit = Convert.ToDecimal(.Item("CreditLimit"))
 End If

 If Not IsDBNull(.Item("InterestRate")) Then
 Me.InterestRate = Convert.ToDecimal(.Item("InterestRate"))
 End If

 If Not IsDBNull(.Item("PaymentFrequency")) Then
 Me.PaymentFrequency = Convert.ToInt32(.Item("PaymentFrequency"))
 End If

 If Not IsDBNull(.Item("CurrentPaymentAmount")) Then
 Me.CurrentPaymentAmount = Convert.ToDecimal(.Item("CurrentPaymentAmount"))
 End If

 If Not IsDBNull(.Item("PaidDate")) Then
 Me.PaidDate = Convert.ToDateTime(.Item("PaidDate"))
 End If
 End With

 Me.BorrowerIDs = GetLoanBorrowerIDs()
 Me.VehicleIDs = GetLoanVehicleIDs()
 Me.RealEstatePropertyIDs = GetLoanRealEstateIDs()
 End Sub

 ''' <summary>
 ''' Gets the details of a Loan record based on the internal ID assigned in the database
 ''' </summary>
 ''' <param name="CurrentLoanID">Database-assigned identification number for a specific Loan record</param>
 Private Sub GetLoanDetail(ByVal CurrentLoanID As Integer)
 Dim MyDB As New SQLDB(DBServer)
 Dim DBData As DataTable

 MyDB.Parameters.Add("requestloanid", CurrentLoanID)
 DBData = MyDB.RunProc("""Loan"".""SelectLoan""", "LoanData")

 If DBData.Rows.Count = 1 Then
 Me.Fill(DBData.Rows(0))
 ElseIf DBData.Rows.Count > 1 Then
 Throw New Exception("More than one record was found with the specified identifier.")
 End If

 MyDB.Dispose()
 DBData.Dispose()
 End Sub

 Private Function GetLoanBorrowerIDs() As List(Of Integer)
 Dim LoanBorrowers As New List(Of Integer)

 If Me.LoanID > 0 Then
 Dim MyDB As New SQLDB(DBServer)
 Dim DBData As DataTable

 MyDB.Parameters.Add("requestloanid", Me.LoanID)
 DBData = MyDB.RunProc("""Loan"".""SelectLoanBorrower""", "LoanBorrowers")

 If DBData.Rows.Count > 0 Then
 For Each LoanBorrower As DataRow In DBData.Rows
 LoanBorrowers.Add(Convert.ToInt32(LoanBorrower("borrowerid")))
 Next LoanBorrower
 End If

 MyDB.Dispose()
 DBData.Dispose()
 End If

 Return LoanBorrowers
 End Function

 ''' <summary>
 ''' Retrieves details about vehicles related to a specific loan as identified by the database-assigned identification number
 ''' </summary>
 ''' <returns></returns>
 Private Function GetLoanVehicleIDs() As List(Of Integer)
 Dim LoanVehicles As New List(Of Integer)

 If Me.LoanID > 0 Then
 Dim MyDB As New SQLDB(DBServer)
 Dim DBData As DataTable

 MyDB.Parameters.Add("requestloanid", Me.LoanID)
 DBData = MyDB.RunProc("""Loan"".""SelectLoanVehicle""", "LoanVehicles")

 If DBData.Rows.Count > 0 Then
 For Each LoanVehicle As DataRow In DBData.Rows
 LoanVehicles.Add(Convert.ToInt32(LoanVehicle("vehicleid")))
 Next LoanVehicle
 End If

 MyDB.Dispose()
 DBData.Dispose()
 End If

 Return LoanVehicles
 End Function

 ''' <summary>
 ''' Retrieves details about real estate properties related to a specific loan as identified by the database-assigned identification number
 ''' </summary>
 ''' <returns></returns>
 Private Function GetLoanRealEstateIDs() As List(Of Integer)
 Dim LoanProperties As New List(Of Integer)

 If Me.LoanID > 0 Then
 Dim MyDB As New SQLDB(DBServer)
 Dim DBData As DataTable

 MyDB.Parameters.Add("requestloanid", Me.LoanID)
 DBData = MyDB.RunProc("""Loan"".""SelectLoanRealEstate""", "LoanRealEstate")

 If DBData.Rows.Count > 0 Then
 For Each LoanProperty As DataRow In DBData.Rows
 LoanProperties.Add(Convert.ToInt32(LoanProperty("realestateid")))
 Next LoanProperty
 End If

 MyDB.Dispose()
 DBData.Dispose()
 End If

 Return LoanProperties
 End Function
#End Region
 End Class
#End Region

NOTE: I've used the EditorBrowsable and DebuggerBrowsable attributes to prevent some properties from popping up in Visual Studio's IntelliSense to avoid confusion when using the Loan object. I've also recently added the DebuggerBrowsableState.Never parameter to most of my class properties to prevent them from attempting to load (and, thus making one of those unnecessary database calls) when I'm debugging the program. They were having some unexpected results while I was debugging, and I honestly couldn't think of a better way to handle them than to just "hide" them unless explicitly called. As I stated above, everything currently seems to be working as intended, but I just can't help feeling like I'm "overdoing" it or, at least, could be doing it better.

Also, I've removed a number of methods (the Insert, Update and Delete methods, for example) from the code below as they aren't really relevant to the specific nature of this question. Thank you for your time.

FULL DISCLOSURE: I am an entirely self-taught programmer. This code actually arose because of a few of the comments I received on a question I asked on the StackOverflow site, "VB.NET How To Prevent Infinite Recursion During Object Population".

General

• Why don't you use Lazy<T> which seems to be perfect for your usage szenario.



• 
  

  Regions, well regions are usually a sign that a class may be too big. If you need regions to structure your code then you have to much code in that class and you should consider to split the class into 2 or more classes.

  
  
  

  At least the description of a region shouldn't lie about its content. Private LenderID As Integer is in the region named PRIVATE PROPERTIES but is clearly a variable.

  
  


• 
  

  Although VB.NET is case-insensitive you should consider to apply the .NET Naming Guidline because readers of your code will usually expect this and it will make it easier for them to read and understand your code.

  
  
  

  E.g methods parameter are named using camelCase casing, classlevel variables are either named using camelCase casing or are underscore prefixed. If you use underscore prefixed variablenames you shouldn't use them with the Me specifier together. Basically you should use Me only if you need to explicitly distinguish between a class level variable and e.g a method parameter which has the same name.

  
  



• You should extract magic strings/numbers into meaningful named constants. Assume you change the name of the requestloanid parameter, it would be much easier to change this only at one place in your code. Right now you would need to change it at least at 4 places in your code.

Property Lender

If one sets the value of this property to Nothing a NullReferenceException will be thrown. Althought that might be acceptable it would be better to throw an ArgumentNullException. IMO parameters of public methods/properties should always be checked at least for Nothing.

Properties Borrowers, Vehicles, RealEstateProperties,

You have a lot of duplicated code in the getters of these properties which should be extracted to separate methods. In addition you don't need to Clear() a New List<T>.

You could think about wether you really want to set a property to Nothing, at least for the getter of the List<T> properties. If there aren't any items in the List<T> why don't you leave them as they are? Doing so would result in not having to check for Nothing from the caller side of the property.

Calling a constructor from an overloaded constructor is more .NET like and the intend of the call is more clear.

Like so

Public Sub New()
 Me.Initialize()
End Sub

Public Sub New(ByVal NewLoanID As Integer)
 Me.New()
 Me.GetLoanDetail(NewLoanID)
End Sub

But reading Me.GetLoanDetail(NewLoanID) made me wonder what a GetXX() method would do if the returned isn't assigned to anything. Thats a clear sign that you should work on your naming. IMO overloading the Fill() method would be a better way. Don't make maintainers of your code wonder what it is doing. Assume you have to add a feature or fix a bug in 3 or 6 months, you would grasp it at first glance what the call is about if you would read Me.Fill(NewLoanID).

GetLoanDetail(), GetLoanBorrowerIDs(), GetLoanVehicleIDs()

Instead of manually disposing the connection and the datatable(which you wouldn't need to dispose btw) you should use a Using statement.

GetLoanBorrowerIDs(), GetLoanVehicleIDs()

By reverting the if condition If Me.LoanID > 0 Then you could return early and would save one level of indentation. Assuming LoanID won't be negative this would look like so

Private Function GetLoanVehicleIDs() As List(Of Integer)
 Dim LoanVehicles As New List(Of Integer)

 If Me.LoanID = 0 Then
 Return LoanVehicles
 End If

 Dim MyDB As New SQLDB(DBServer)
 Dim DBData As DataTable

 MyDB.Parameters.Add("requestloanid", Me.LoanID)
 DBData = MyDB.RunProc("""Loan"".""SelectLoanVehicle""", "LoanVehicles")

 If DBData.Rows.Count > 0 Then
 For Each LoanVehicle As DataRow In DBData.Rows
 LoanVehicles.Add(Convert.ToInt32(LoanVehicle("vehicleid")))
 Next LoanVehicle
 End If

 MyDB.Dispose()
 DBData.Dispose()

 Return LoanVehicles
End Function