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

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 2

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).