Let’s say I have an antique repair shop and I can repair antiques that people bring in or I can arrange the collection and then delivery of antiques for repair.
For my Domain Model I have a Repair class and a CourierJob class the Repair Class is concerned with everything regarding the repair of an antique and the CourierJob class knows only about validating a courier tracking code and formatting a URL so that users
can track a consignment.
I have design the class like this:
Public Class RepairJob
Private _Collection
As CourierJob
Private _Delivery
As CourierJob
Private _ID
As Long
Public
Sub New(ByVal ID
As Long)
_ID = ID
' Initialise the courier jobs with their default behaviour (no tracking code)
' When the Repair Job is retrieve from the database we can set the
' individual properties
_Collection = New CourierJob()
_Delivery = New CourierJob()
End Sub
Public
ReadOnly Property Collection()
As CourierJob
Get
Return _Collection
End
Get
End Property
Public
ReadOnly Property Delivery()
As CourierJob
Get
Return _Delivery
End
Get
End Property
' Other properties/methods.......
End Class
Public
Class DAL
Public
Shared Function GetARepairJob(ByVal ID
As Long)
As RepairJob
' Example of how I might rehyrdate the entity
Dim aRepairJob
As RepairJob
Dim reader
As IDataReader
' Go to database and get the data......
aRepairJob = New RepairJob(reader("ID"))
aRepairJob.Collection.TrackingCode = reader("CollectionCode").ToString
aRepairJob.Delivery.TrackingCode = reader("DeliveryCode").ToString
Return aRepairJob
End Function
End
Class
Is this a good design or would it better to have the CourierJobs as properties on the Repair Job with Getters/Setters?
The only thing I've ever concluded on is that it depends, but I don't think I can be consistent on what it depends on :p
A third, not mentioned, option will be to construct the CourierJobs first, and pass them into the constructor of the RepairJob. I personally don't like the setting the Collection.TrackingCode like in the sample, but it's 50/50 if passing into a constructor
is any better than setting courier job properties.
You get better encapsulation not "chaining" these setters like this, but I'm not committing to that as the best solution.
I'm also not convinced by the explanation that RepairJob should depend on CourierJob like this in the first place. Is it not rather that a CourierJob depends on a Repair (i.e. invert the relationship)?
My preference would be to have any of the List<T> (CourierJob) collections lazy-loaded from the BLL, so each instance in the hierarchy above would be loaded at table level, rather than at an psuedo aggregate level as you appear to be
doing in the DAL above. I can probably (later) drop off a quick example in c# of what i mean but am kinda heading off for a bit at the moment, so sorry for giving half the tale just now and leaving you in 'suspense' :[)
Thanks for your speedy responses benhart and jimibt [Yes].
Hi jimibt, I guess putting the CourierJobs into a list does make sense, so I have changed my code below - I would still like to see the example that you were talking about if you have the time.
benhart I have also changed the construtor of the Courier job so that is takes a RepairJob as a reference so that the RepairJob no longer depends on the CourierJob. Does this seem like a better solution?
Public Enum DeliveryType
Delivery = 1
Collection = 2
End Enum
Public
Class RepairJob
Private _ID
As Long
Private _CourierJobs
As List(Of CourierJob)
Public
Sub New(ByVal ID
As Long)
_ID = ID
End Sub
Public
Sub AddCourierJob(ByVal TrackingCode
As String,
ByVal DeliveryType
As DeliveryType)
' I can always check here if I want a 1 collection/delivery rule
CourierJobs.Add(New CourierJob(RepairJob, TrackingCode, DeliveryType)
End Sub
Public
Sub RemoveCourierJob(ByVal CourierJob
As CourierJob)
CourierJobs.Remove(CourierJob)
End Sub
Public
Function GetCourierJobs()
As ReadOnlyCollection(Of CourierJob)
Return CourierJobs.AsReadOnly
End Function
Private
Function CourierJobs()
As List(Of CourierJob)
If _CourierJobs
Is Nothing
Then
' Could Lazy Load here...
_CourierJobs = New List(Of CourierJob)
End
If
Return _CourierJobs
End Function
' Other properties/methods.......
End Class
But my original question still remains unanswered.. i think, hmm lets try a slightly different take on it... give me a second I have to put the dinner on.
Likewise, 'tea' now finished at this end [:)]. ok, i've uploaded a quick n dirty app to demontrate my thinking on this. if you just skip to the App_Code\BLL\RepairSchedule\ folder, then the basic classes are there. Most of the 'work' is inside the RepairDetails.cs
class (this is the top of the aggregate heirarchy).
I 'hope' that everything is self explanatory, but if any questions, then get them in quick as the 1st beer is about to be popped in 5 mins. (oh, and in vs2008, either right click on Admin\Default.aspx and
set as start page or once it comes up in the browser, click on the Admin 'link)
[edit] - as it's just CRUD forms in the example, i should explain that in a REAL app, you'd be looking at the
Repair form as the master record and the CourierRepair form as the details below this. the way i've modelled it allows you to have multiple pickup/returns per Repair (if required) - this may or may not be desirable but hopefully will demonstrate
the concept.
btw - occassionally, there can be issues with downloads that are zips (security settings), if you have issues, navigate to the App_Data folder and set the security settings on those files to be 'safe' (or whatever the parlance is on the properties tab
for it). i've seen this happen a few times, causing the app not to run with an erroneous error message.
The speed with which one can build all that scaffolding is quite compelling.
(In case you're curious, I'm still looking through it, having been dragged in a few directions lately. Likely be able to dedicate more time this weekend.)
But my original question still remains unanswered.. i think, hmm lets try a slightly different take on it... give me a second I have to put the dinner on.
Right I have a rather contrived example...
Lets say I have a problem and that problem can have a solution category with a number of steps, now I may want to change the solution category but I would like to keep the steps I have added to complete the solution.
Version 1:
Public
Class Problem
Private _Solution
As Solution
Private _Description
As
String
Private _ID
As
Long
Public
Sub
New(ByVal Id
As
Long)
_ID = Id
_Solution = New Solution()
End
Sub
Public
Property Description()
As
String
Get
Return _Description
End
Get
Set(ByVal value
As
String)
_Description = value
End
Set
End
Property
Public
Property Solution()
As Solution
Get
Return _Solution
End
Get
Set(ByVal value
As Solution)
_Solution = value
End
Set
End
Property
End Class
Public
Class Solution
Private _SolutionSteps
As List(Of ISolutionStep)
Private _SolutionDesc
As
String =
"Not Set"
Public
Sub
New()
_SolutionSteps = New List(Of ISolutionStep)
End
Sub
Public
Property Description()
As
String
Get
Return _SolutionDesc
End
Get
Set(ByVal value
As
String)
_SolutionDesc = value
End
Set
End
Property
Public
ReadOnly
Property Steps()
As ReadOnlyCollection(Of ISolutionStep)
Get
Return _SolutionSteps.AsReadOnly
End
Get
End
Property
Public
Sub AddStep(ByVal aStep
As ISolutionStep)
_SolutionSteps.Add(aStep)
End
Sub
End Class
Version 2:
Public Class Problem
Private _SolutionSteps
As List(Of ISolutionStep)
Private _SolutionDesc
As
String =
"Not Set"
Private _Description
As
String
Private _ID
As
Long
Public
Sub
New(ByVal Id
As
Long)
_ID = Id
_SolutionSteps = New List(Of ISolutionStep)
End
Sub
Public
Property Description()
As
String
Get
Return _Description
End
Get
Set(ByVal value
As
String)
_Description = value
End
Set
End
Property
Public
Property SolutionCategory()
As
String
Get
Return _SolutionDesc
End
Get
Set(ByVal value
As
String)
_SolutionDesc = value
End
Set
End
Property
Public
ReadOnly
Property Steps()
As ReadOnlyCollection(Of ISolutionStep)
Get
Return _SolutionSteps.AsReadOnly
End
Get
End
Property
Public
Sub AddStep(ByVal aStep
As ISolutionStep)
_SolutionSteps.Add(aStep)
End
Sub
End Class
Now I know that version 2 doesn't quite make sense because a problem should only be concerned with dealing with the problem and not dealing with the solution, but version 1 doesn't make sense eiether. How would you model this?
The speed with which one can build all that scaffolding is quite compelling.
(In case you're curious, I'm still looking through it, having been dragged in a few directions lately. Likely be able to dedicate more time this weekend.)
Ben,
yes, my keyboard has in fact melted, so this may be last message... [:)]!!
re looking thro.. I know exactly how it is, i'm a 'fits n starts' guy (after hours). there's no rush but hopefully this little example may ease you into the 'frame'(work) given it's context in the current discussion. subliminally, you'll almost be
there. If you've got any comments re the structure (of this example) that i proposed, drop the broad case and i'll quickly throw up another version later (time permitting).
But my original question still remains unanswered.. i think, hmm lets try a slightly different take on it... give me a second I have to put the dinner on.
Right I have a rather contrived example...
Lets say I have a problem and that problem can have a solution category with a number of steps, now I may want to change the solution category but I would like to keep the steps I have added to complete the solution.
[cut for brevity]
Now I know that version 2 doesn't quite make sense because a problem should only be concerned with dealing with the problem and not dealing with the solution, but version 1 doesn't make sense eiether. How would you model this?
Scott - see the example i dropped and 'imagine that the Repair is the Problem and the
CourierRepair is the Solution. This structure will allow you to add as many steps (or CoutrierRunTypes) as you want. The actual final step would have a bit field called AcceptedSolution (or similar) and would be set to true.
If you need to see that exemplified, let me know and i can have a 'crude' version up in around 10-15 mins [:)]
I 'hope' that everything is self explanatory, but if any questions, then get them in quick as the 1st beer is about to be popped in 5 mins. (oh, and in vs2008, either right click on Admin\Default.aspx and
set as start page or once it comes up in the browser, click on the Admin 'link)
Cool I will take a look at it before the mighty Portsmouth continue on their european tour! I'm on my 2nd san miguel.. your on catch up!
scott@elband...
Star
11346 Points
1865 Posts
Advice on a design and the single reponsiblity principle
Oct 02, 2008 03:22 PM|LINK
Let’s say I have an antique repair shop and I can repair antiques that people bring in or I can arrange the collection and then delivery of antiques for repair.
For my Domain Model I have a Repair class and a CourierJob class the Repair Class is concerned with everything regarding the repair of an antique and the CourierJob class knows only about validating a courier tracking code and formatting a URL so that users can track a consignment.
I have design the class like this:
Public Class RepairJob Private _Collection As CourierJob
Private _Delivery As CourierJob
Private _ID As Long Public Sub New(ByVal ID As Long)
_ID = ID
' Initialise the courier jobs with their default behaviour (no tracking code)
' When the Repair Job is retrieve from the database we can set the
' individual properties
_Collection = New CourierJob()
_Delivery = New CourierJob()
End Sub
Public ReadOnly Property Collection() As CourierJob
Get
Return _Collection
End Get
End Property Public ReadOnly Property Delivery() As CourierJob
Get
Return _Delivery
End Get
End Property ' Other properties/methods.......
End Class
Public
Class DALPublic Shared Function GetARepairJob(ByVal ID As Long) As RepairJob
' Example of how I might rehyrdate the entity
Dim aRepairJob As RepairJob
Dim reader As IDataReader ' Go to database and get the data......
aRepairJob = New RepairJob(reader("ID"))
aRepairJob.Collection.TrackingCode = reader("CollectionCode").ToString
aRepairJob.Delivery.TrackingCode = reader("DeliveryCode").ToString
Return aRepairJob
End Function
End
Class Is this a good design or would it better to have the CourierJobs as properties on the Repair Job with Getters/Setters?benhart
Participant
818 Points
179 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 04:07 PM|LINK
To set or not to set. Always a good question :)
The only thing I've ever concluded on is that it depends, but I don't think I can be consistent on what it depends on :p
A third, not mentioned, option will be to construct the CourierJobs first, and pass them into the constructor of the RepairJob. I personally don't like the setting the Collection.TrackingCode like in the sample, but it's 50/50 if passing into a constructor is any better than setting courier job properties.
You get better encapsulation not "chaining" these setters like this, but I'm not committing to that as the best solution.
I'm also not convinced by the explanation that RepairJob should depend on CourierJob like this in the first place. Is it not rather that a CourierJob depends on a Repair (i.e. invert the relationship)?
I'm interested to hear others' opinions...
jimibt
Member
724 Points
185 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 04:38 PM|LINK
Hi Scott,
My preference would be to have any of the List<T> (CourierJob) collections lazy-loaded from the BLL, so each instance in the hierarchy above would be loaded at table level, rather than at an psuedo aggregate level as you appear to be doing in the DAL above. I can probably (later) drop off a quick example in c# of what i mean but am kinda heading off for a bit at the moment, so sorry for giving half the tale just now and leaving you in 'suspense' :[)
Back in a few hrs....
jimi
scott@elband...
Star
11346 Points
1865 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 05:05 PM|LINK
Thanks for your speedy responses benhart and jimibt [Yes].
Public Enum DeliveryTypeHi jimibt, I guess putting the CourierJobs into a list does make sense, so I have changed my code below - I would still like to see the example that you were talking about if you have the time.
benhart I have also changed the construtor of the Courier job so that is takes a RepairJob as a reference so that the RepairJob no longer depends on the CourierJob. Does this seem like a better solution?
Delivery = 1
Collection = 2
End Enum
Public
Class RepairJobPrivate _ID As Long
Private _CourierJobs As List(Of CourierJob)
Public Sub New(ByVal ID As Long)
_ID = ID
End Sub
Public Sub AddCourierJob(ByVal TrackingCode As String, ByVal DeliveryType As DeliveryType)
' I can always check here if I want a 1 collection/delivery rule
CourierJobs.Add(New CourierJob(RepairJob, TrackingCode, DeliveryType)
End Sub Public Sub RemoveCourierJob(ByVal CourierJob As CourierJob)
CourierJobs.Remove(CourierJob)
End Sub Public Function GetCourierJobs() As ReadOnlyCollection(Of CourierJob)
Return CourierJobs.AsReadOnly
End Function Private Function CourierJobs() As List(Of CourierJob)
If _CourierJobs Is Nothing Then
' Could Lazy Load here...
_CourierJobs = New List(Of CourierJob)
End If
Return _CourierJobs
End Function ' Other properties/methods.......
End Class
But my original question still remains unanswered.. i think, hmm lets try a slightly different take on it... give me a second I have to put the dinner on.
jimibt
Member
724 Points
185 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 07:10 PM|LINK
Ok Scott,
Likewise, 'tea' now finished at this end [:)]. ok, i've uploaded a quick n dirty app to demontrate my thinking on this. if you just skip to the App_Code\BLL\RepairSchedule\ folder, then the basic classes are there. Most of the 'work' is inside the RepairDetails.cs class (this is the top of the aggregate heirarchy).
I 'hope' that everything is self explanatory, but if any questions, then get them in quick as the 1st beer is about to be popped in 5 mins. (oh, and in vs2008, either right click on Admin\Default.aspx and set as start page or once it comes up in the browser, click on the Admin 'link)
ok, let me know how you get on:
Scott's Repair Courier example that may or may not be correct in jimi's little mind
[edit] - as it's just CRUD forms in the example, i should explain that in a REAL app, you'd be looking at the Repair form as the master record and the CourierRepair form as the details below this. the way i've modelled it allows you to have multiple pickup/returns per Repair (if required) - this may or may not be desirable but hopefully will demonstrate the concept.
btw - occassionally, there can be issues with downloads that are zips (security settings), if you have issues, navigate to the App_Data folder and set the security settings on those files to be 'safe' (or whatever the parlance is on the properties tab for it). i've seen this happen a few times, causing the app not to run with an erroneous error message.
jimi
benhart
Participant
818 Points
179 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 07:31 PM|LINK
Jeepers jimi, you type fast [:)]
The speed with which one can build all that scaffolding is quite compelling.
(In case you're curious, I'm still looking through it, having been dragged in a few directions lately. Likely be able to dedicate more time this weekend.)
scott@elband...
Star
11346 Points
1865 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 07:39 PM|LINK
Right I have a rather contrived example...
Lets say I have a problem and that problem can have a solution category with a number of steps, now I may want to change the solution category but I would like to keep the steps I have added to complete the solution.
Version 1:
Public
Class ProblemPrivate _Solution As Solution
Private _Description As String
Private _ID As Long Public Sub New(ByVal Id As Long)
_ID = Id
_Solution = New Solution()
End Sub Public Property Description() As String
Get
Return _Description
End Get
Set(ByVal value As String)
_Description = value
End Set
End Property Public Property Solution() As Solution
Get
Return _Solution
End Get
Set(ByVal value As Solution)
_Solution = value
End Set
End Property
End Class
Public
Class SolutionPrivate _SolutionSteps As List(Of ISolutionStep)
Private _SolutionDesc As String = "Not Set" Public Sub New()
_SolutionSteps = New List(Of ISolutionStep)
End Sub Public Property Description() As String
Get
Return _SolutionDesc
End Get
Set(ByVal value As String)
_SolutionDesc = value
End Set
End Property Public ReadOnly Property Steps() As ReadOnlyCollection(Of ISolutionStep)
Get
Return _SolutionSteps.AsReadOnly
End Get
End Property Public Sub AddStep(ByVal aStep As ISolutionStep)
_SolutionSteps.Add(aStep)
End Sub
End Class
Version 2:
Public Class Problem
Private _SolutionSteps As List(Of ISolutionStep)
Private _SolutionDesc As String = "Not Set"
Private _Description As String
Private _ID As Long
Public Sub New(ByVal Id As Long)
_ID = Id
_SolutionSteps = New List(Of ISolutionStep)
End Sub Public Property Description() As String
Get
Return _Description
End Get
Set(ByVal value As String)
_Description = value
End Set
End Property Public Property SolutionCategory() As String
Get
Return _SolutionDesc
End Get
Set(ByVal value As String)
_SolutionDesc = value
End Set
End Property Public ReadOnly Property Steps() As ReadOnlyCollection(Of ISolutionStep)
Get
Return _SolutionSteps.AsReadOnly
End Get
End Property
Public Sub AddStep(ByVal aStep As ISolutionStep)
_SolutionSteps.Add(aStep)
End Sub
End Class
Now I know that version 2 doesn't quite make sense because a problem should only be concerned with dealing with the problem and not dealing with the solution, but version 1 doesn't make sense eiether. How would you model this?
jimibt
Member
724 Points
185 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 07:44 PM|LINK
Ben,
yes, my keyboard has in fact melted, so this may be last message... [:)]!!
re looking thro.. I know exactly how it is, i'm a 'fits n starts' guy (after hours). there's no rush but hopefully this little example may ease you into the 'frame'(work) given it's context in the current discussion. subliminally, you'll almost be there. If you've got any comments re the structure (of this example) that i proposed, drop the broad case and i'll quickly throw up another version later (time permitting).
jimi
jimibt
Member
724 Points
185 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 07:48 PM|LINK
Scott - see the example i dropped and 'imagine that the Repair is the Problem and the CourierRepair is the Solution. This structure will allow you to add as many steps (or CoutrierRunTypes) as you want. The actual final step would have a bit field called AcceptedSolution (or similar) and would be set to true.
If you need to see that exemplified, let me know and i can have a 'crude' version up in around 10-15 mins [:)]
jimi
scott@elband...
Star
11346 Points
1865 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 07:49 PM|LINK
Cool I will take a look at it before the mighty Portsmouth continue on their european tour! I'm on my 2nd san miguel.. your on catch up!