Design problem #2
This is a subset of the Back to the Checkout kata by Dave Thomas, which I used many times as a TDD training exercise.
Suppose you have a PriceRules object that knows that the prices of items. Its responsibility is to know the following table:
Item Unit Price Special Price A 50 3 for 130 B 30 2 for 45 C 20 D 15
Then you have a Cart object that knows which items a customer is trying to buy. For instance, a given cart could contain the list [A, A, C, A, D].
The problem is to compute the total that the customer has to pay, out of a collaboration between (at least) the cart and the pricerRules objects. It seems easy, but there is a catch: you are forbidden to use getters. All methods must return “void”, in Java terms. Design the messages that are exchanged between the objects and produce the desired result. (In the example, it would be 165). Have fun!
June 14th, 2010 at 17:36
Too easy to pass a Collector parameter or bypass getter with setter?
June 15th, 2010 at 08:12
Collector parameter is how I would solve it. I don’t know what “bypass getter with setter” is.
June 15th, 2010 at 08:53
a setFullPrice on the Cart, and a fillPrice(Cart) on the PriceRules that call the setFullPrice. Another way to see the collector parameter. In this way the two class are more coupled, I dont know it is better or worse (can dipende)
June 15th, 2010 at 10:24
thank you for publishing this exercise, I will get back with some source code soon (the tyme to copy it in pastie.org :)
June 15th, 2010 at 10:40
I see… you are implementing the responsibilities of the collecting parameter in the cart. I don’t mean it’s bad; the point of the exercise is to discuss alternatives! And the real test of the design comes when you see how well it handles the next changes :-)
June 15th, 2010 at 20:03
l’avevo sotttovalutato.
questo il risultato: http://www.pastie.org/1005668
ora voglio rivedere la relazione tra le tre classi Cashier, Bill e PriceRules.
ottimo codice per un coding dojo sul TDD, grazie ancora!
commenti e suggerimenti per migliorare il codice sono ben accetti.
June 16th, 2010 at 08:06
Luca: riesci a riformulare i metodi che si chiamano HandleXYZ e le classi che si chiamano XYZHandler senza usare la parola “Handle(r)” e in modo che il nome spieghi che cosa fanno?
June 16th, 2010 at 10:25
something like:
public class Bill: IBill
public void VisitBillGrandTotal(IBillGrandTotalVisitor visitor)
{
visitor.VisitGrandTotal(_grandTotal);
}
?
June 16th, 2010 at 16:45
Luca,
I still don’t find these names communicate your intent. What I mean is that a name like handleDetails() does not say much. It’s like calling a method doSomething(). Names like addAmountToTotal() or registerUser() or sumOfLineItems() may express intent.
Your code contains three classes, but does not contain a class for the Cart. (I see you use a List<string> instead.)
There is nowhere in your code a test that proves that if I buy A, A, B, A the total will be 150 (for instance).
I see complex logic, with many IFs. It’s difficult to follow what the “previousItemCount += 1” do.
June 16th, 2010 at 17:52
for the last 3 points I agree that needs to be improved and I know how to do it.
But for the first one … I tried hard but was not capable to come up with a better name. can you suggest me a better one ?
June 16th, 2010 at 18:07
How about WriteGrandTotalOn()
or WriteGrandTotalTo() ?
June 17th, 2010 at 23:42
yep, good suggestions. thanks
June 18th, 2010 at 09:44
good suggestions. thank you.
June 27th, 2010 at 12:51
Sorry for being so late..
Here’s mine, not complete – just one test case – because I wanted to concentrate on pushing on refactoring:
* http://pastie.org/1020661
No “getters” at all:
* price is calculated with
anItem.priceGiven(priceRules)* total is sent to display with
sum.displayResultOn(display)June 27th, 2010 at 18:23
Jacopo:
I like …displayResultOn(display). But:
– why the Cart builds the rules with “new” instead of receiving them? That would force the Cart to, say, open a connection to the database if the rules are stored there.
– anItem.priceGiven is a sort of getter
– anItem.priceGiven breaks completely when you consider special offers
June 28th, 2010 at 13:50
Hi matteo:
1) short anwser is: there is not yet no tests for that. Having an external factory or using default constructor, so far, are equivalent
2) not really: it encapsulates item’s “type”, which otherwise whould be exposed through a getter. priceRules stores rules, items know how to achieve prices for their “type”
3) yes, you’re probably right!
As I told in the previous comment, I just wanted to focus on your question about computing total using Cart and PriceRule collaborating, with no getter at all. (I’ve done this kata a few times, but wasn’t able to find anywhere on my laptop..)
I’ll try going further and share the results with you,
ciao.
August 17th, 2010 at 01:21
I agree with Matteo about point #2, priceGiven() is just a getter and nothing else, so the “you are forbidden to use getters” rule is broken.
I encourage Jacopo and Luca to expose a more “OO Design” point of view before coding the answer.
If no getters must be used, then you have to use “ByRef” style parameters in both Cart and PriceRules, because otherwise you won’t be able to know what items from the Cart are selected by the user, or what is the price from PriceRules of each item.
So, my first approach is as follow:
1. Cart must implement a method that receives an empty List of 3 Object references per item selected (Item, Quantity, and Subtotal), and this method will summarize the entire Cart resulting in a List as follow (for the example):
Item: A, C, D
Qty.: 3, 1, 1
SubT: null, null, null
2. Then, PriceRules must implement a method that receives the resulting List (if not empty) filling the third element for every item with the corresponding computed subtotal.
3. This two functions must be called inside a third method implemented in a third class that handle the request for computation of the “total that the customer has to pay”, receiving the Cart and the PriceRules to use, and then iterate over the resulting List (completely filled) to sum every subtotal and then resolving how to show, return, store, or delegate the result handling to another class.