Reputation: 15807
Please take a look at the code below:
Imports System.Data.SqlClient
Public Class Person
Private id As String
Private name As String
Public Function check(ByVal personid As String) As Boolean
'Do some checks on the person to see if it is ready for deletion
End Function
Public Shared Sub Delete()
Dim v As New Vehicle
Dim p As New Person
End Sub
End Class
Public Class Vehicle
Private vrm As String
Public Function check(ByVal vehicleid As String) As Boolean
'Do some checks on the vehicle to see if it is ready for deletion
End Function
Private Shared Sub Delete()
Dim p As New Person
Dim v As New Vehicle
Dim objCommand As SqlCommand
Dim objCon As SqlConnection
Dim objDR As SqlDataReader
Try
Dim _ConString As String = "Data Source=IANSCOMPUTER;Initial Catalog=Test;Integrated Security=True;MultipleActiveResultSets=true"
objCon = New SqlConnection(_ConString)
objCommand = New SqlCommand("SELECT * FROM Person were startdate < dateadd(year,-6," & Now & ")")
objDR = objCommand.ExecuteReader
Do While objDR.Read
If p.check(objDR("id")) And v.check(objDR("vehicleid")) Then
'Execute delete statement, which deletes the person and vehicle
End If
Loop
objDR.Close()
objCommand.Connection = objCon
objCon.Open()
objCommand.ExecuteNonQuery()
Catch ex As Exception
Throw
Finally
End Try
End Sub
End Class
Notice that the shared function (Person.Delete) contains references to Person and Vehicle and uses instance variables in Person and Vehicle. Basically a check needs to be done on the Person and vehicle before the person and vehicle can be deleted.
Is it poor practice to reference Person and Vehicle from the Shared function? Is it poor practice to use instance variables in the Delete function. The Delete function will delete thousands of persons and vehicles daily.
Upvotes: 0
Views: 314
Reputation: 2437
It's not necessarily a bad practice to make them shared, but if they are shared its a good practice to refactor them into their own classes. Shared
instantly makes then global functions that, in my mind, should not contaminate the class signature.
So this would be better practice in my estimation:
Public Class Person
Private id As String
Private name As String
End Class
Public Class Vehicle
Private vrm As String
End Class
Public Class PersonVanisher
Public Shared Sub Delete()
Dim v As New Vehicle
Dim p As New Person
...
End Sub
Public Shared Function Check(personId As String) As Boolean
...
End Function
End Class
Public Class VehicleCrusher
Private Shared Sub Delete()
Dim p As New Person
Dim v As New Vehicle
...
End Sub
Public Shared Function Check(vehicleId As String) As Boolean
...
End Function
End Class
People get into philosophical debates about this, but if it were my code I would not make anything shared. I would prefer to "New up" a VehicleCrusher
when I need it and let .NET dispose everything as soon as I'm done.
Upvotes: 1