Business logic in contructor - C# / Linq to Sql

Question

I'm extending (not sure if it's the right word here) a partial Cart class that got generated in Linq to SQL database model.

The business logic is that there can be only one Cart per customer. If a customer doesn't have a cart, it should be created; if a customer has a cart, it should be returned.

Here's what I'm doing:

public partial class Cart
{
        //the rest of the Cart class is in the .dbml file created by L2S
        public Cart(int userId)
        {
            Cart c = GetCurrentCart(userId);
            this.CartId = c.CartId ;
            this.UserId = c.UserId;
        }

        public Cart GetCurrentCart(int userId)
        {
            Cart currentCart = new Cart();

            // if cart exists - get it from DB
            //if not - create it, save in DB, and get if right out
            //all of this is done with Linq to SQL

            return currentCart;

        }
}

calling a method from a constructor just doesn't seem right, am I enforcing the BL the right way?

Answer 1

I'd question why the "Cart" class is so smart. In Domain Driven Design terms, it sounds like the User "owns" the Cart. So why not something like:

var user = // Load a user
var cart = user.Cart;

In this case your Cart getter could lazily load/initialize the cart.

Answer 2

I agree with Paul Stovell, it sounds like user should own the cart. But at any rate, by the time your constructor is called you already have a new instance of Cart. C# doesn't let you alter the reference returned by the constructor, so rather than clients of the Cart class using the constructor they should be calling a static factory method (I don't have experience with Linq to SQL, so this may not work directly).

Your GetCurrentCart method is almost this; you just need to mark it static. Additionally, you should make the Cart constructor just responsible for creating a new Cart and make it private so that clients are forced to use GetCurrentCart. An implementation might look something like this:

public partial class Cart
{
        // Make a new cart
        private Cart(int userId, int cartId)
        {
            this.CartId = userId;
            this.UserId = cartId;
        }

        private static Dictionary<int, Cart> CurrentCarts = new Dictionary<int, Cart>();

        public static Cart GetCurrentCart(int userId)
        {
            // TODO: Use a proper caching mechanism that will at least
            //       remove old carts from the dictionary.
            Cart cart;
            if (CurrentCarts.TryGetValue(userId, out cart))
            {
                return cart;
            }

            cart = /* try get cart from DB */;
            if (cart == null)
            {
                // Make a new cart
                cart = new Cart(userId, GenerateCartId());
            }

            CurrentCarts[userId] = cart;

            return cart;
        }
}