Is having a lot of properties initiated at instantiation a good practice?

Question

I have a Cart class that has a lot of properties:

public class Cart
{
    public Customer Customer { get; set; }
    public Products Products { get; set; }
    public Payments Payments { get; set; }
    public DateTime TimeOfArrival { get; set; }
    //...and so on
}

Whenever I want to instantiate a new Cart, I need to initiate these properties. Some of them are already known (e.g. Customer), others not yet (e.g. Products and Payment). So this is what I ended up doing this when I instantiate a new ticket:

Cart Cart = new Cart
                    {
                        // for know properties:
                        Customer = currentCustomer,
                        TimeOfArrival = DateTime.Now,

                        // for properties that are going to be filled later:
                        Products = new List<Product>(),
                        Payments = new List<Payment>(),

                        // ...and so on
                    };

This solution obviously works but seems very bulky at instantiation, plus there's a lot of work done in initiator which I assume is a bad thing.

Am I doing this right or is having a lot of properties initiated at instantiation not a good practice ? If so, how should I do it ?

Thanks in advance

Nothing wrong per se. Indeed setting up defaults for your properties is a good practice. You can shorten your code though with https://stackoverflow.com/questions/40730/how-do-you-give-a-c-sharp-auto-property-a-default-value This will remove the need of repeating lines of code when you initialize your objects — Steve, May 05 '19 at 20:48
Why cant the Cart class instantiate those objects in the ctor? Eliminates the need for the consumer to do so in order for anything to work correctly. — Ňɏssa Pøngjǣrdenlarp, May 05 '19 at 20:59
@Aldert @Steve Thanks for your quick answers, there are actually a handful of other properties which I spared you though I believe it doesn't change much. The problem is the constructor won't let me set `new List()` as default parameter value in the constructor. — Marcel Barc, May 05 '19 at 20:59

Stefan · Accepted Answer · 2019-05-05T21:06:37.430

Yes, there is room for improvement. But be careful, a lot depends on requirements, and basically they should be leading in a lot of cases. And it's a matter of taste. Some people are more puristic than others.

It depends on your project: how important is it? Is it a side project or your core business?

And, yes there is room for improvement; there always is... now, next year, at every review... etc. Just saying: there is no perfect design.

Now, my strongly biased opinion : )

Your setup represents a typical case in a domain driven design. I'll send a link later.

In general, in domain driven design you try to determine a specific business functionality/concept, and wrap this in, preferably a most descriptive object. Basically you are on the right track with your cart.

But, it seems the cart has just a bit to much domain knowledge in it.

Specifically the nested complex types can be an indication that you are crossing some domain boundaries.

For example: the customer ; which is situated within the cart. The customer might in its turn contain an address. ... and so on.

Clearly one's address is irrelevant for the cart itself.

Also; payment. I dont think its typically bound to a cart, maybe more to an order.

As for the products; a small description and a link to the actual product would suffice. Next to the obvious price and quantity ofcourse.

For a cart, you might want to consider to store a customer ID only, and fetch the customer if additional data is required.

Anyhow, it's a though subject in general. The best advise is to; not try to involve too much complex types, and only put in your model the thing that only makes sense to that specific "thing"

Thank you for your very inspiring answer. I'm actually trying to make a UWP registering app for my own (very small) business, though it doesn't have to be very sophisticated. — Marcel Barc, May 05 '19 at 21:08
Cool :-) ... a bonus: if you're UWP, its definitely worth to checkout the MVVM pattern : ) — Stefan, May 05 '19 at 21:19
I'm already on it ! coming from data science I feel like I'm discovering a brand new world in oop. — Marcel Barc, May 05 '19 at 21:26

Patrick Artner · Answer 2 · 2019-05-06T05:46:34.093

Use a constructor:

public class Cart
{
    // shorthand constructor, only 1 setting, else use a normal function style constructor
    public Cart (Customer cust) => Customer = cust;

    public Customer Customer { get; set; }
    public Products Products { get; set; } = new List<Product>();
    public Payments Payments { get; set; } = new List<Payment>();
    public DateTime TimeOfArrival { get; set; } = DateTime.Now;
    //...and so on
}

var cart = new Cart( my_customer_instance );  // all thats needs to be done

This is easier then the Lazy<...> approach if you only develop something for your own business. Lazy rocks if you have costly things to create that might or might not be needed at all. Empty lists do not cost you much - you could even go as far as creating them with 0 elements if you are really concerned (They'll increase internal buffer if you "feed" them).

The big advantage is that you do not care what to do if you need a new Cart(customer) - the class handles initialization itself.

I'm definitely going to use that approach, I didn't know about this way of defining default values for properties. — Marcel Barc, May 05 '19 at 21:28

score 1 · Answer 3 · answered May 05 '19 at 20:56

If you are worried about the cost of initializing properties up front (which, in this case, doesn't look like a problem), then you can use Lazy<T> to conveniently instantiate the properties when they are needed.

For example

private Lazy<List<Product>> products;
constructor() {
   products = new Lazy<List<Product>>(()=>new List<Product>());
} 
public List<Product> Products => products.Value;

For something like a list, this is pretty silly, but for a more expensive intializating, Lazy<T> can be useful.

Not really what I was looking for but good to know, thanks ! Actually my Product class is quite heavy so might be useful in the future. — Marcel Barc, May 05 '19 at 21:04

Is having a lot of properties initiated at instantiation a good practice?

3 Answers3