1

Design and implement Cash Register:

Given a number of items you will be required to calculate the total bill. Items are charged for in a couple of different ways:

  • A given price for each item, e.g. Boxes of Cheerios are $6.99 each

  • A given price by weight, e.g. Apples are $2.49 per pound

  • Items can go on sale where you might receive bulk discounts, e.g. Buy two get one free sales; Buy four get one free

  • Coupons are available to get money off the bill when the total amount is above a threshold, e.g. $5 off when you spend $100 or more

My answer: I am not sure how much is correct, or how I can improve it?!

I attached my design based on C#, and the code can be view on https://github.com/mandanemedia/CashRegister.

I appreciated if you could help to understand Inheritance is better or Composition. Based on the given scenario the answer must have the following concrete classes:

  • ProductSellByQuantity.cs
  • ProductSellByQuantityInGroupSell.cs
  • ProductSellByWeight.cs

As well as two objects ShoppingCart & ShoppingCartByCoupon or might be only a ShoppingCart object.

My goal is to make OO design skills & design pattern better.

Click on it to open in bigger size. ProductSellSection ShoppingCart

Improve this question
3
  • 1
    I used the strategy pattern to implement this kata. It worked out pretty well. I created an PricingStrategy interface and then implemented things like PerPoundStrategy and XForXStrategy. The hard part was actually to recalculate as each item was scanned, as the same UPC can be scanned "out of order" with another item(s) in between.
    – RubberDuck
    Jul 14, 2016 at 1:32
  • @RubberDuck - I've used the same approach in POS implementations too. Having a list of objects that are given access to scan through your collection of real purchases and add additional virtual line items for discounts, shipping, taxes, and so on is a useful approach here.
    – Jules
    Jul 14, 2016 at 9:11
  • Migrated from CodeReview. Interesting. Perhaps the rules should be amended to allow review of design. In this case it looks complete enough to benefit from a review.
    – radarbob
    Jul 16, 2016 at 0:25

1 Answer 1

Reset to default
2

Premature Over-Engineering

For example, I assert that you do not need any of those interfaces. If needed later, make them later.

An interface (the C# keyword kind) is for giving common behavior to unrelated classes and then handle objects polymorphically.

Think more carefully about what constitutes a more specific class. Think of what a shopping cart IS. Is a "coupon shopping cart" a more specific kind of cart, or is it simply an attribute in a cart?

Think more carefully about how things work together, in contrast to seeing everything as separate, distinct bits. A coupon is a logical part of calculating a price. GetPrice is simply a necessary "grunt" method in a product class.

"groupsale" is a volume discount. This is the same concept as a coupon. Both are discounts. Said another way, they are simply adjustments to the price. Therefore they are an integral part of calculating cost. They are not classes.

Too much interface

Code to interfaces not implementation does not mean make everything a (C# keyword) interface. Any class' set of public methods IS an interface.

Implementing interfaces instead of inheriting from an abstract class sets you up for lots of redundant implementation.

This is a grotesque (sorry) mis-application of the Interface segregation principle. I cannot see how ISell justifies itself as a distinct interface. I do not see ISell references being passed around. GetPrice() simply looks like it naturally belongs to a ProductXXX object; where its meaning is more clear inside of a functional class.

Ditto for the shopping cart.

Weight, Quantity Abstracted

Abstracted, I strongly suspect the multiple ProductSellBy... classes coallece into one.

These are simply values. And these values are used in calculation in the same way, i.e. GetPrice() * Quantity() The fact that quantity represents countable things or weight can be captured by a separate property. You've already got an enum for this, make a property of that type in the Product class.

This abstraction does away with the motivation for the many interfaces and classes and your diagram will shrink by more than half I think.

DRY up redundant methods

Make an abstract class for your Products.

ShoppingCart coupon

"By coupon" makes no sense as a shopping cart sub-class. A coupon is simply an adjustment to price calculation.

As above, get rid of those interfaces and make an abstract class if you must sub-class ShoppingCart.

And when you think about it, a coupon is not really a special case. A cost is coupon-discounted the same way for any item, I assume. So that discount can be built in to the price calculation. This is accomodated by the coupon's default value that would represent no discount.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.