Should I incorporate list of fees/discounts into an order class or have them be itemlines

Question

I have no other developers to ask for advice or "what do you think - I'm thinking this" so please, if you have time, have a read and let me know what you think.

It's easier to show than describe, but the app is essentially like a point of sale app with 3 major parts: Items, OrderItems and the Order.

The item class is the data as it comes from the datastore.

public class Item 
    : IComparable<OrderItem>, IEquatable<OrderItem>
{
    public Int32 ID { get; set; }
    public String Description { get; set; }
    public decimal Cost { get; set; }

    public Item(Int32 id, String description, decimal cost) 
    { 
        ID = id; 
        Description = description; 
        Cost = cost;
    }
    // Extraneous Detail Omitted
}

The order item class is an item line on an order.

public class OrderItem 
    : Item, IBillableItem, IComparable<OrderItem>, IEquatable<OrderItem>
{
    // IBillableItem members
    public Boolean IsTaxed { get; set; } 
    public decimal ExtendedCost { get { return Cost * Quantity; } } 

    public Int32 Quantity { get; set; }

    public OrderItem (Item i, Int32 quantity) 
        : base(i.ID, i.Description, i.Cost) 
    {
        Quantity = quantity;

        IsTaxed = false;
    }
    // Extraneous Detail Omitted
}

Currently when you add fees or discounts to an order it's as simple as:

Order order = new Order();
// Fee
order.Add(new OrderItem(new Item("Admin Fee", 20), 1));

// Discount
order.Add(new OrderItem(new Item("Today's Special", -5), 1));

I like it, it makes sense and a base class that Order inherits from iterates through the items in the list, calculates appropriate taxes, and allows for other Order-type documents (of which there are 2) to inherit from the base class that calculates all of this without re-implimenting anything. If an order-type document doesn't have discounts, it's as easy as just not adding a -$ value OrderItem.

The only problem that I'm having is displaying this data. The form(s) that this goes on has a grid where the Sale items (ie. not fees/discounts) should be displayed. Likewise there are textboxes for certain fees and certain discounts. I would very much like to databind those ui elements to the fields in this class so that it's easier on the user (and me).

MY THOUGHT

Have 2 interfaces: IHasFees, IHasDiscounts and have Order implement them; both of which would have a single member of List. That way, I could access only Sale items, only Fees and only Discounts (and bind them to controls if need be).

What I don't like about it: - Now I've got 3 different add/remove method for the class (AddItem/AddFee/AddDiscount/Remove...) - I'm duplicating (triplicating?) functionality as all of them are simply lists of the same type of item, just that each list has a different meaning.

Am I on the right path? I suspect that this is a solved problem to most people (considering that this type of software is very common).

Answer 1

One option is to add a ItemType attribute to OrderItem

enum ItemType
{
   Item,
   Fee,
   Discount
}

Now you could in your order class have:

public IList<OrderItem> Fees
{
   get
   {
      return _items.Find(i=>i.ItemType==ItemType.Fee);
   }
}

Now you can still keep your single list and avoid the extra interfaces. You could even have a method like IList GetItems(ItemType type).

One other thought is your current design doesn't allow for a discount of a %. Today you get 10% off. This might not be a requirement, but one option to avoid the application having to calculate this is to seperate the items from the discounts.

The discounts could even become more of rules, if I order 10 items take 5% off.

Answer 2

I'll point you to a remark by Rob Connery on an ALT.net podcast I listened to not long ago (I'm not an ALT.net advocate, but the reasoning seemed sound):

What does make sense to a "business user" (if you have any of those around).

As a programmer, you're gonna want to factor in Item, Fee, Discount etc, because they have similar attributes and behaviors.

BUT, they might be two totally separate concepts in terms of the model. And someone is gonna come at a later time, saying "but this makes no sense, they are separate things, I need to report on them separately and I need to apply this specific rule to discounts in that case".

DRY does not mean limiting your model, and you should keep that in sight when factoring behavior via inheritance or anything like that.

The specific example that was used in that case was that of the shopping cart. The programmer's natural idea was to use an order in an uncommited state. And it makes sense, because they look exactly the same. Except that they are not. It makes no sense to the client, because they are two separate concept, and it just make the design less clear.

It is a matter of practices, taste and opinion though, so don't blindly follow advice posted on a web site :)

And to your specific problem, the system I work with uses items, fees, line-item discount (a property of the item) and a global discount on the order (though it's not an order, it's POS receipt but it does not really matter in that case).

I guess the reason is that, behind those concepts, Items are specific instances of inventoried pieces, they impact stock quantities, they are enumerable and quantifiable.

Fees are not. They do not share most of the attributes.

It might not matter in your case, because your domain seems much more limited than that, but you might want to keep those issues in mind.

Answer 3

Effectively, I'd look at your design in the details and try to figure out where the behaviors lie; then extract any commonalities in those behaviors to a distinct interface and make sure that applies to your design.

To wit; Fees may have associated validation behaviors associated with them. Let's say you add a Fee to any Order which has 20 items or more (just a random example, run with me on this one). Now, when you add the 20th item, you may want to add that Fee to the Order, but there's a problem; when you remove an item from your order, do you want to have to check every time to see if you need to remove that Fee from your order? I doubt it; the implication here is that there is a behavior that is associated with the Fees / Discounts that essentially makes them an entirely different class of things.

I'd look at it this way; categorize Fees and Discounts as "Special" things, and then create an "ISpecial" interface from which both Fees and Discounts inherit. Extract any common functionality to the ISpecial interface (for example, "Validate"). Then have your Order implement the ISpecial (or whatever) interface.

In that way, you can define the specific Fee.Validate() behavior and the Discount.Validate behavior, and have the operate properly thanks to the magic of polymorphism (foreach of m_specialCollection .validate those). In that way, as well, you can easily extend the Special interface for anything else that might be necessary (say, Taxes).

Answer 4

I think the core of the problem that you're facing here is that you've implemented OrderItem as a subclass of Item, and now you're discovering that this really isn't always appropriate.

Given what you describe, here's how I'd try implementing this:

Create an Order class that implements public properties for every single-valued data element that you want to expose to data binding: order number, date, customer, total fees, total discounts, etc. It sounds like you may be needing to display specific fees/discounts as single values; if so, implement public properties for those.

Create an abstract OrderItem class that implements public properties for every data element that you want to bind to in the grid, and for every data element that you want to sort the items on. (You could also make this an IOrderItem interface; it really depends on whether or not there are going to be methods common to all order items.)

Create subclasses of OrderItem (or classes that implement IOrderItem) for the specific kinds of line item that can appear on an order: ProductOrderItem, FeeOrderItem, DiscountOrderItem, etc.

In your implementation of ProductItem, implement a property of type Item - it'd look something like:

public class ProductItem : OrderItem
{
   public Item Item { get; set; }
   public string Description { get { return Item.Description; } }
   public int Quantity { get; set; }
   public decimal Amount { get { return Item.Price * Quantity; } }
}

Implement a property of type IEnumerable<OrderItem> within Order for storing all of the line items. Implement an AddItem method for adding OrderItems, e.g.:

public void AddItem(OrderItem item)
{
   _Items.Add(item); // note that backing field is a List<OrderItem>
}

which you can call pretty simply:

Order o = new Order();
o.AddItem(new ProductOrderItem { Item = GetItem(1), Quantity = 2 });
o.AddItem(new FeeItem { Description = "Special Fee", Amount = 100 });
o.AddItem(new DiscountItem { DiscountAmount = .05 });

Write implementations of those single-valued fields that need to extract values from this list, e.g.:

public decimal TotalFees
{
   get
   {
      return (from OrderItem item in Items
              where item is FeeItem
              select item.Amount).Sum();
   }
}

You can come back later and optimize these properties if necessary (e.g. saving the computation once you've done it once).

Note that you could also restrict AddItem to adding ProductItems, and use other methods in the Order to add other types of items. For instance, if an order can have only one discount amount:

public void SetDiscountAmount(decimal discountAmount)
{
   DiscountOrderItem item = _Items
      .Where(x => x is DiscountOrderItem)
      .SingleOrDefault();
   if (item == null)
   {
       item = new DiscountOrderItem();
       _Items.Add(item);
   }
   item.DiscountAmount = discountAmount;
}

You'd use this approach if you wanted to display the discount amount in the appropriate place in the grid of order items, but also wanted an order's discount amount to be a single value. (It's arguable that you might want to make DiscountAmount a property of the Order, create the DiscountOrderItem in its setter, and have DiscountOrderItem get its Amount from Order.DiscountAmount. I think both approaches have their pros and cons.)