Bloody brits. with all those beers, it's no wonder you lost the empire [:)]. At least we still respect the queen's english this side, else I'd really feel left out the party.
I've been jumping in and out of your problem, scott, trickier than it seemed. My more domain driven (but not DDD probably) approach has a lot more classes, but they interact together. It would be my first take, but seems ok. I still think there is some confusion
between steps, resolutions and categories (a step doesn't result in the claim being resolved, the group of steps represented by the category does). I thus created ClaimResolution, which the claim has a history of, and which can be marked as resolution. I think
you might have something closer to ResolutionStatus on this (not the boolean), where previous resolutions are marked as not successful, current one is pending, and successful on, successful. Claim would probably have a property of the current 'resolution',
likely the pending one pending. You could also, i spose, separate CurrentResolution, from ResolutionHistory.
internal class ClaimController
{
public void TakeStepsToResolve()
{
var claim = new Claim();
//select the appropriate category...
ClaimResolutionCategory category = new SendBikeForRepairs();
//add resolution to claim...
claim.AddResolution(category);
}
}
internal class Claim
{
private List _claimResolutions = new List();
public string Customer { get; set; }
public void AddResolution(ClaimResolutionCategory category)
{
ClaimResolution resolution = new ClaimResolution(category);
foreach (var step in category.CategorySteps)
{
step.Perform(this);
}
_claimResolutions.Add(resolution);
}
public void Resolve()
{
_claimResolutions.Last().MarkAsResolution();
}
}
internal class ClaimResolution
{
private DateTime _created;
private bool _successful;
private ClaimResolutionCategory _category;
public ClaimResolution(ClaimResolutionCategory category)
{
_created = DateTime.Now;
_category = category;
}
public bool Successful
{
get { return _successful; }
}
public void MarkAsResolution()
{
_successful = true;
}
}
internal abstract class ClaimResolutionCategory
{
public abstract IList CategorySteps { get; }
}
internal class SendBikeForRepairs : ClaimResolutionCategory
{
public override IList CategorySteps
{
get { return new List { new PartTakenOutOfStock(), new SendPart() }; }
}
}
internal interface IStep
{
void Perform(Claim claim);
}
internal class PartTakenOutOfStock : IStep
{
public void Perform(Claim claim)
{
//remove stock based on claim data?
}
}
internal class SendPart : IStep
{
public void Perform(Claim claim)
{
//notify shipping based on claim data
}
}
I've also recently started on the DDD 'journey', so let me know what you think...
gonna head off now but just wanted to say, i enjoyed the evenings banter and in particular, (Ben) liked your
finale step (or should i say IStep). nice and elegant and used exactly as required. nice one. as an aside, i think i now design with the domain in mind as the previous examples that i uploaded were all
contrived with the domain model to the fore (i.e. in my head i knew what FK's i wanted to apply to which entities to achieve my goal in the aggregate).
Yeah thanks for all your input guys and the banter, I will run through all of the code tomorrow and let you know how I get on, its getting late now and .........
Yeah thanks for all your input guys and the banter, I will run through all of the code tomorrow and let you know how I get on, its getting late now and .........
I've been jumping in and out of your problem, scott, trickier than it seemed. My more domain driven (but not DDD probably) approach has a lot more classes, but they interact together.
Thats a great solution benhart. From the UI point of view I want to interact with it like this (in reality these steps would happen over several days)
Dim claim As Claim =
New Claim()
' Right so the customer rings up the shop and says "my bike is broke",
' the shop estbalishes what they are going to do, lets say the shop decides to send out a part.
' So the Resolution Category would be set to "Send out a part for the customer to repair".
claim.ResolutionCategory = New SendBikeOutForRepairResolution
' Two steps would be added to Resolution called "Part Taken Out Of Stock" and "Part sent".
claim.ResolutionCategory.Steps.add(New PartTakenOutOfStock)
claim.ResolutionCategory.Steps.add(New PartSent)
' At any point we need to tell the step what part was taken out of stock
CType(claim.ResolutionCategory.Steps(0), PartTakenOutOfStock).Product = Product
CType(claim.ResolutionCategory.Steps(1), PartSent).Order = Order
' Now the customer gets the part and tries to fit it to the bike but can't so the
' shop decides to ask the customer to take the Bike and part to a garage so that they can fit it.
' The Resolution category changes to "Take the bike to a motor bike garage for repair"
claim.ResolutionCategory = New RepairAtShopResolution
' and another step is added to the resolution called "Bike sent to garage, invoice required".
claim.ResolutionCategory.Steps.add(New SentForRepairInvoiceRequired)
' But we don't want to loose the previous two steps as they relate to the resolution of the claim.
' Some time passes...
' Then else where in the code we check to see if the claim is complete. The claim is only complete if
' all the steps are deemed to be complete. i.e. The PartTakenOutOfStock step has a product associated with it etc
If claim.ResolutionCategory.IsComplete
Then
Me.lblMessage.text =
"Claim complete"
Else
Dim errors
As New System.Text.StringBuilder
For Each BR
As BrokenRule
In claim.ResolutionCategory.GetBrokenRules
errors.Append(BR.Name)
Next
Me.lblMessage.text =
"Errors<br/>:" & errors.ToString
End If
I will try and put together a small demo app of what I am trying to get to so that you can download and check it out.
' Right so the customer rings up the shop and says "my bike is broke",
' the shop estbalishes what they are going to do, lets say the shop decides to send out a part.
' So the Resolution Category would be set to "Send out a part for the customer to repair".
claim.ResolutionCategory = New SendBikeOutForRepairResolution
' Two steps would be added to Resolution called "Part Taken Out Of Stock" and "Part sent".
claim.ResolutionCategory.Steps.add(New PartTakenOutOfStock)
claim.ResolutionCategory.Steps.add(New PartSent)
' At any point we need to tell the step what part was taken out of stock
CType(claim.ResolutionCategory.Steps(0), PartTakenOutOfStock).Product = Product
CType(claim.ResolutionCategory.Steps(1), PartSent).Order = Order
I don't think you should do this downcasting here. It assumes, firstly that you're storing in a list that keeps the order that they were added, and that there were no other items in there (too many details of implementation are exposed). You should probably
configure the steps properly before they end up in claim, and the interface should define and functionality that they need once added. Since those steps depend on the category, perhaps the category must enforce that required data is passed in through it's
contructor, adding the category to the claim means they're automatically associated.
scott@elbandit.co.uk
' Now the customer gets the part and tries to fit it to the bike but can't so the
' shop decides to ask the customer to take the Bike and part to a garage so that they can fit it.
' The Resolution category changes to "Take the bike to a motor bike garage for repair"
claim.ResolutionCategory = New RepairAtShopResolution
Back to the original debate of setting, I personally would prefer making this more explicit in the form of a method. Especially for referenced objects, it prevents people setting willy-nilly.
scott@elbandit.co.uk
I will try and put together a small demo app of what I am trying to get to so that you can download and check it out.
I'm heading out for a bit, will take a look later. Look forward to it, it's been a fun exercise.
' Now the customer gets the part and tries to fit it to the bike but can't so the
' shop decides to ask the customer to take the Bike and part to a garage so that they can fit it.
' The Resolution category changes to "Take the bike to a motor bike garage for repair"
claim.ResolutionCategory = New RepairAtShopResolution
Back to the original debate of setting, I personally would prefer making this more explicit in the form of a method. Especially for referenced objects, it prevents people setting willy-nilly.
Yeah this is the piece of the puzzle I am having problems with [8-)]. I have greatly simplified the design in the download.
I was trying some single responsibility exercises:
"The Claim can resolve itself" - does this makes sense? Or should we have an object like a resolution then have the sentance "A resolution can resolve a claim".
Anyway have a look at the down and many, many, many thanks again!
"The Claim can resolve itself" - does this makes sense? Or should we have an object like a resolution
then have the sentance "A resolution can resolve a claim".
I agree. A resolution can resolve a claim. This does seem better. In the original code I posted this was the other way around, but I see now that is wrong. A claim can be marked as resolved, though, and in the process I suppose needs to be associated with the
resolution that resolved it.
Looking at your code, the Resolution becomes the more important entity, but I think it might not be named appropriately. It's more like a ClaimActivityTracker or something. The name resolution applies a certain finality, it was the action taken that solved
the problem. Once the customer has confirmed that the last actions taken did the trick, maybe there is a method on the "activity tracker" that resolves, and sets some information on the claim, identifying what the resolution was. Looking at the diagram, this
class correctly holds that place in the centre. (In your test, once you've created the claim, it quickly becomes forgotten about, and all subsequent actions are against the "aClaim.Resolution.". This was the main clue to me.)
Also, when we were discussing categories earlier, they seemed to come preloaded with a few steps. Unfortunately enums in .net don't really facilitate this, and their easy use tempts us to use them when we actually need something else. Perhaps the category is
a class that has some actions associated? Perhaps there is another class which, based on passed in data, performs some steps. While demonstrative rather than your final product, the test has a lot of logic in it, which would likely repeat in other places.
This logic might need to be encapsulated into something, but you probably know that.
Also, in your example the enum is changed explicitly on the resolution, immediately losing the history of previous categories. Perhaps this should be a list too. Then you step into the temporal conundrum, though: the category is explicitly linked to the
steps taken to fulfill it - you have a few lists that represent this history, but not the relationship between them. You need to, in this case, create a class that groups these things together, and rather store a list of these objects, giving the real history
of what happened... (I've got a thing for the temporal patterns, having recently been enlightened. Forgive me if this sounds like an overkill, but when data changes with time our models need to cater for it, and represent it accurately. This is not always
appropriate, but in your case it might be.)
So my wording would be: A customer can phone in and log a claim. A claim activiity tracker is created to track all history of the claim. Actions are taken to attempt to resolve the claim, typically part of predefined categories, and include courier jobs
and orders. Once the customer has confirmed that the claim is resolved, the tracker is informed as much, and the actions which resolved the claim are identified.
benhart
Participant
818 Points
179 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 10:25 PM|LINK
Bloody brits. with all those beers, it's no wonder you lost the empire [:)]. At least we still respect the queen's english this side, else I'd really feel left out the party.
I've been jumping in and out of your problem, scott, trickier than it seemed. My more domain driven (but not DDD probably) approach has a lot more classes, but they interact together. It would be my first take, but seems ok. I still think there is some confusion between steps, resolutions and categories (a step doesn't result in the claim being resolved, the group of steps represented by the category does). I thus created ClaimResolution, which the claim has a history of, and which can be marked as resolution. I think you might have something closer to ResolutionStatus on this (not the boolean), where previous resolutions are marked as not successful, current one is pending, and successful on, successful. Claim would probably have a property of the current 'resolution', likely the pending one pending. You could also, i spose, separate CurrentResolution, from ResolutionHistory.
internal class ClaimController { public void TakeStepsToResolve() { var claim = new Claim(); //select the appropriate category... ClaimResolutionCategory category = new SendBikeForRepairs(); //add resolution to claim... claim.AddResolution(category); } } internal class Claim { private List _claimResolutions = new List(); public string Customer { get; set; } public void AddResolution(ClaimResolutionCategory category) { ClaimResolution resolution = new ClaimResolution(category); foreach (var step in category.CategorySteps) { step.Perform(this); } _claimResolutions.Add(resolution); } public void Resolve() { _claimResolutions.Last().MarkAsResolution(); } } internal class ClaimResolution { private DateTime _created; private bool _successful; private ClaimResolutionCategory _category; public ClaimResolution(ClaimResolutionCategory category) { _created = DateTime.Now; _category = category; } public bool Successful { get { return _successful; } } public void MarkAsResolution() { _successful = true; } } internal abstract class ClaimResolutionCategory { public abstract IList CategorySteps { get; } } internal class SendBikeForRepairs : ClaimResolutionCategory { public override IList CategorySteps { get { return new List { new PartTakenOutOfStock(), new SendPart() }; } } } internal interface IStep { void Perform(Claim claim); } internal class PartTakenOutOfStock : IStep { public void Perform(Claim claim) { //remove stock based on claim data? } } internal class SendPart : IStep { public void Perform(Claim claim) { //notify shipping based on claim data } }I've also recently started on the DDD 'journey', so let me know what you think...
jimibt
Member
724 Points
185 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 10:36 PM|LINK
Ben/Scott - yes, what a glorious ermmm(pire) [:)]
gonna head off now but just wanted to say, i enjoyed the evenings banter and in particular, (Ben) liked your finale step (or should i say IStep). nice and elegant and used exactly as required. nice one. as an aside, i think i now design with the domain in mind as the previous examples that i uploaded were all contrived with the domain model to the fore (i.e. in my head i knew what FK's i wanted to apply to which entities to achieve my goal in the aggregate).
funny old world - yawn (or is that hick)
ok -see y'all anon!!
jimi
scott@elband...
Star
11346 Points
1865 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 02, 2008 10:52 PM|LINK
Yeah thanks for all your input guys and the banter, I will run through all of the code tomorrow and let you know how I get on, its getting late now and .........
Gooooooaaaaaalllll.....You Beauty!!!!!!!!!!!!!!!
2349: GOAL Guimaraes 2-1 Portsmouth (agg 2-3)
jimibt
Member
724 Points
185 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 03, 2008 10:56 AM|LINK
Scott,
meant to mention last night (re flixon), all the relevant info can be found in the announcement thread on here:
Announce: Flixon Site Generator V2 now available
and well done pompey (and peter crouch of course) - skin o' the teeth stuff!!
jimi
scott@elband...
Star
11346 Points
1865 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 03, 2008 12:53 PM|LINK
Thats a great solution benhart. From the UI point of view I want to interact with it like this (in reality these steps would happen over several days)
Dim claim As Claim = New Claim()
' Right so the customer rings up the shop and says "my bike is broke",
' the shop estbalishes what they are going to do, lets say the shop decides to send out a part.
' So the Resolution Category would be set to "Send out a part for the customer to repair".
claim.ResolutionCategory = New SendBikeOutForRepairResolution ' Two steps would be added to Resolution called "Part Taken Out Of Stock" and "Part sent".
claim.ResolutionCategory.Steps.add(New PartTakenOutOfStock)
claim.ResolutionCategory.Steps.add(New PartSent) ' At any point we need to tell the step what part was taken out of stock
CType(claim.ResolutionCategory.Steps(0), PartTakenOutOfStock).Product = Product
CType(claim.ResolutionCategory.Steps(1), PartSent).Order = Order ' Now the customer gets the part and tries to fit it to the bike but can't so the
' shop decides to ask the customer to take the Bike and part to a garage so that they can fit it.
' The Resolution category changes to "Take the bike to a motor bike garage for repair"
claim.ResolutionCategory = New RepairAtShopResolution ' and another step is added to the resolution called "Bike sent to garage, invoice required".
claim.ResolutionCategory.Steps.add(New SentForRepairInvoiceRequired) ' But we don't want to loose the previous two steps as they relate to the resolution of the claim.
' Some time passes... ' Then else where in the code we check to see if the claim is complete. The claim is only complete if
' all the steps are deemed to be complete. i.e. The PartTakenOutOfStock step has a product associated with it etc
If claim.ResolutionCategory.IsComplete Then
Me.lblMessage.text = "Claim complete"
Else
Dim errors As New System.Text.StringBuilder
For Each BR As BrokenRule In claim.ResolutionCategory.GetBrokenRules
errors.Append(BR.Name)
Next Me.lblMessage.text = "Errors<br/>:" & errors.ToString
End If
I will try and put together a small demo app of what I am trying to get to so that you can download and check it out.
scott@elband...
Star
11346 Points
1865 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 03, 2008 05:07 PM|LINK
Ok jimibt & benhart how about this for a start off?
BikeClaim.zip
I have tried to simplify the domain but I know its still very smelly, please could you give me your opinion and some advice [:D]
I very much appreciate all your help!
Scott
benhart
Participant
818 Points
179 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 03, 2008 05:14 PM|LINK
It's the weekend!
But, lucky for you, I'm also a workaholic (or hobbyist, which sounds better) [:)] I'll check it out tomorrow.
I feel obliged to point out that you must be one of the few DDD exponents in VB, quite a rare breed I'm sure [:D]. Nice one.
benhart
Participant
818 Points
179 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 04, 2008 09:21 AM|LINK
hey scott
just a few interim comments...
I don't think you should do this downcasting here. It assumes, firstly that you're storing in a list that keeps the order that they were added, and that there were no other items in there (too many details of implementation are exposed). You should probably configure the steps properly before they end up in claim, and the interface should define and functionality that they need once added. Since those steps depend on the category, perhaps the category must enforce that required data is passed in through it's contructor, adding the category to the claim means they're automatically associated.
Back to the original debate of setting, I personally would prefer making this more explicit in the form of a method. Especially for referenced objects, it prevents people setting willy-nilly.
I'm heading out for a bit, will take a look later. Look forward to it, it's been a fun exercise.
scott@elband...
Star
11346 Points
1865 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 04, 2008 10:32 AM|LINK
Yeah this is the piece of the puzzle I am having problems with [8-)]. I have greatly simplified the design in the download.
I was trying some single responsibility exercises:
"The Claim can resolve itself" - does this makes sense? Or should we have an object like a resolution then have the sentance "A resolution can resolve a claim".
Anyway have a look at the down and many, many, many thanks again!
benhart
Participant
818 Points
179 Posts
Re: Advice on a design and the single reponsiblity principle
Oct 04, 2008 12:50 PM|LINK
I agree. A resolution can resolve a claim. This does seem better. In the original code I posted this was the other way around, but I see now that is wrong. A claim can be marked as resolved, though, and in the process I suppose needs to be associated with the resolution that resolved it.
Looking at your code, the Resolution becomes the more important entity, but I think it might not be named appropriately. It's more like a ClaimActivityTracker or something. The name resolution applies a certain finality, it was the action taken that solved the problem. Once the customer has confirmed that the last actions taken did the trick, maybe there is a method on the "activity tracker" that resolves, and sets some information on the claim, identifying what the resolution was. Looking at the diagram, this class correctly holds that place in the centre. (In your test, once you've created the claim, it quickly becomes forgotten about, and all subsequent actions are against the "aClaim.Resolution.". This was the main clue to me.)
Also, when we were discussing categories earlier, they seemed to come preloaded with a few steps. Unfortunately enums in .net don't really facilitate this, and their easy use tempts us to use them when we actually need something else. Perhaps the category is a class that has some actions associated? Perhaps there is another class which, based on passed in data, performs some steps. While demonstrative rather than your final product, the test has a lot of logic in it, which would likely repeat in other places. This logic might need to be encapsulated into something, but you probably know that.
Also, in your example the enum is changed explicitly on the resolution, immediately losing the history of previous categories. Perhaps this should be a list too. Then you step into the temporal conundrum, though: the category is explicitly linked to the steps taken to fulfill it - you have a few lists that represent this history, but not the relationship between them. You need to, in this case, create a class that groups these things together, and rather store a list of these objects, giving the real history of what happened... (I've got a thing for the temporal patterns, having recently been enlightened. Forgive me if this sounds like an overkill, but when data changes with time our models need to cater for it, and represent it accurately. This is not always appropriate, but in your case it might be.)
So my wording would be: A customer can phone in and log a claim. A claim activiity tracker is created to track all history of the claim. Actions are taken to attempt to resolve the claim, typically part of predefined categories, and include courier jobs and orders. Once the customer has confirmed that the claim is resolved, the tracker is informed as much, and the actions which resolved the claim are identified.
Make any sense?