views:

94

answers:

2

I am creating a simple aspnetmvc cart application and have defined classes similar to the following:

public class Cart
{
    public Guid Id { get; set; }
    public string Comment { get; set; }
    public DateTime CreatedOn { get; set; }
    public DateTime UpdatedOn { get; set; }
    public DateTime? DeletedOn { get; set; }
    public List<CartItem> CartItems { get; set; }
}

public class CartItem
{
    public Guid Id { get; set; }
    public Guid CartId { get; set; }
    public string Sku { get; set; }
    public double ItemAmount { get; set; }
    public double Amount { get; set; }
    public int Quantity { get; set; }
}

With a very simple repository that looks something like this:

public interface ICartRepository
{
    Cart CreateCart();
    Cart GetCart(Guid Id);
    Cart UpdateCart(Cart cart);
    void DeleteCart(Cart cart);
}

After creating the classes, I begin to have the sense that I would be better suited to break out the List property from the Cart class and then recombine them in my view model.

public class vmCart
{
    public Cart cart { get; set; }
    public List<CartItem> CartItems { get; set; }
    public string CreatedOn
    {
        get
        {
            return cart.CreatedOn.ToString();
        }
    }
    public string CartTotal
    {
        get
        {
            var total = (double)0;
            foreach (var lineItem in CartItems)
            {
                total += lineItem.Amount;
            }
            return total.ToString("c");
        }
    }
}

It would mean I would have to add additional methods to my model for CRUD of CartItems, but would still allow me to present the object to the view (through the viewmodel) as a combined entity.

There may be no clear advantages to either format, but I would love any feedback on the design.

Best regards,

Hal

+3  A: 

I'd stick with just having a Cart in your model and reference it's items through Cart.CartItems. The moment you start trying to do something in two places you are opening yourself to duplication of code.

Don't violate Kent Beck's "Once and once only rule."

Your models only purpose is to give everything to the view it needs. Your business and data logic should not be in there.

Just my two cents and good luck!

Kindness,

Dan

Daniel Elliott
+5  A: 

Personally I would keep the CartItems in the Cart. Here's why:

  1. There is a clear "has" relationship that suggests the Cart has CartItems in it.

  2. The Cart is a clear Aggregate in your domain model. It is unlikely that you will be loading up a cart entity without also loading up the cart items. While this makes your CRUD ops a bit more complex in your repository, it is the expected behaviour of any repository consumer to do an UpdateCart() rather than perform its own iteration and do UpdateCartItem().

  3. Breaking it out into another object adds complexity that has no distinctive purpose

  4. It is just as easy to get the items into your View either way.

If anything changes about your domain model or the way you deal with your cart items, these assumptions could change and hence the approach you take. But that's how I see it at the moment.

womp
The first two answers were both terrific and came to basically the same conclusion. I've accepted the quickest response. Thanks for all the help...
Hal