2

Please see the class below:

public class Customer
    {
        private readonly IList<Order> _orders = new List<Order>();

        public string FirstName { get; set; }
        public string LastName { get; set; }
        public string Province { get; set; }
        public IEnumerable<Order> Orders { get { return _orders; } } //This line

        internal void AddOrder(Order order)
        {
            _orders.Add(order);
        }
}

As it stands; a person can only have one order assigned to them (there is a good reason for this as it stands - if the same person creates another order then another order is created). However, this could change in the future.

I was recently asked why I am using a list in the domain model to represent orders. There are two options:

1) Use a list on the basis that in the future the plan is to have one person with multiple orders.

2) Use an object for now and change it to a list in future if needed.

I realise this is a bit pedantic. However, I am working for an organisation that likes to use the principle of least astonishment. Which option should I be going with; with regards to the above. Is there a principle at stake here?

4
  • 1
    What would be the impact in the rest of your code if you decide to use a single value now, but have to change it later to a collection? If it's an implementation details, why do you even have to expose this to the rest of your code? Can't you just follow the Tell, Don't Ask principle and just encapsulate whatever choice you make in the class? Feb 19 '18 at 13:52
  • 2
    if the same person creates another order then another order is created That doesn't really answer whether a person can only have one order or not. As a counterexample: if a person has an order from months ago; and today wishes to place a second order; do you intend to delete the old order and overwrite it with the second one? Because that's what's going to happen if you only allow a person to have one order in your data structure.
    – Flater
    Feb 19 '18 at 14:02
  • Related.
    – Robbie Dee
    Feb 19 '18 at 15:49
  • 1
    Ask stakeholders or business analysts about the chances for a user to have more than one order (now or in a near future). Requirements are not clear enough for us to say what you should do. My "arachnid sense" is telling me "go with the collection" (biased by many similar situations where customer and orders were involved).
    – Laiv
    Feb 19 '18 at 16:24
4

If you were to follow YAGNI on this you would have to defer implementing anything in the customer class concerning orders. Once you start adding anything on orders you introduce something that will break if you change it later.

So you have to make a decision right now is it just an object or is it something that will be a list when the system is used.

The decision is in this case very easy. Even if a customer at most can have one order, there is still the case where the customer do not have any orders.

An empty list is always cleaner than NULL.

And even if all customers always have exactly one order, if the requirements/user stories tell you that, it will change.

3
  • +for "An empty list is always cleaner than NULL". Can you provide a reference for this?
    – w0051977
    Feb 20 '18 at 10:02
  • 1
    @w0051977 See Effective Java, #43, "Return empty arrays or collections, not nulls"
    – user949300
    Feb 20 '18 at 20:45
  • @user949300 that quote is about not returning NULL when returning a collection. My point is about avoiding having a property being NULL as you then will have to check against NULL everywhere you use it. That should really be the responsibility of the class having the property, not everybody else.
    – Bent
    Feb 21 '18 at 10:50
3

Although David Arno mentioned a good point (avoiding unneeded complexity), I don't fully agree.

There are many factors that should be considered before making that decision, and to me one of the most important is:

How much work will it mean to later change from object to list?

  • How many classes will rely on the fact that there is only one order per customer? These classes will need to be revisited with that change.
  • Is there a database structure that has to be changed, needing to migrate existing data? Is it only your company's data or do you have to repeat that database migration for multiple sites, or produce a reliable script that will do an automated migration?

And (although of course I don't know your application case) I'd question the "fact" that one customer only has one order. Most businesses like their customers to come back with new orders quite often, and even allow them to have multiple orders being processed in parallel.

So you should estimate:

  • the added complexity of a list over a single object
  • the workload produced by later changing from object to list
  • the likelihood that a list will be necessary later.
1
  • I agreed. Refactoring models from single objects to collection (IMO) is harder and more expensive than the opposite. Having a collection for a single instance could be inefficient but it's a single solution solving both cases
    – Laiv
    Feb 19 '18 at 16:27
3

While it is good to predict possible future requirements, you must consider the extra complexity you are adding to your software. Complexity that at current point in time isn't necessary. Instead of adding unnecessary complexity to your software in hopes that one day it will pay off, I'd rather keep the code as simple and clean as possible and only implement what is needed. If every design decision you make is based on "what changes the future MIGHT bring", you will end up with an over engineered piece of software, that is unnecessarilly hard to read and understand and ironically hard to maintain and expand.

2

Should I use a collection property if I only require an object at the moment?

Probably not; given that you don't need the extra flexibility now, you are gambling that

  • you have correctly guessed what the future requirement will be
  • the cost of changing the API later exceeds the extra cost that the flexibility imposes now
  • that this implementation will survive long enough for the slow bets to pay off

You are right to be thinking in advance about future changes, but you may be missing a key idea: that you want future changes that extend the current functionality, rather than changes that disrupt it.

You current guess is that you are eventually going to want an Orders function, so in thinking about the current API you want to leave a hole so that you can easily add that method later.

interface V1 {
    Order Order();
}

interface V2  : V1 {
    IEnumerable<Order> Orders();
}

Now, when the requirements for returning multiple orders appears, you'll need to figure out which of those orders the V1 interface should return. You could try to make that guess today, and arrange the V1 interface to match

interface V1 {
    Order LatestOrder();
}

Or (recommended) you can wait until the situation is more clear

interface V1 {
    Order Order(); // delegates to V2.LatestOrder
}

interface V2  : V1 {
    Order LatestOrder();
    IEnumerable<Order> Orders();
}

The point being that clients can continue to use the V1 interface with the V2 implementation -- the changes are backwards compatible.

0

Avoid adding complexity now in anticipation of a future requirement. The future has an annoying habit of turning out differently to our predictions.

If you code in support for multiple orders and that feature is never needed, your code will be more complex than it needs to be, for all time (well for its lifetime at least). You will have wasted development time implementing that unused feature, and you'll have broken the principle of least astonishment by having a weird, more complex than it needs to be, feature in the code that'll potentially slow down maintenance as folk work out it's there "just in case", not because it's needed.

Instead, ensure your code is kept simple (only implement what's needed now), and well supported by unit tests. That way, if multiple orders are needed in future, it becomes a relatively simple task to refactor the code to support that feature.

3
  • 2
    It's often referred to as YAGNI ("You aren't gonna need it").
    – mgthomas99
    Feb 19 '18 at 14:00
  • Unfortunately, since the Order (or List of Orders) looks to be exposed via a public getter, much more than Customer will need to be refactored on a change, because all the classes using Customer will "know" the decision. See [why Getters are Evil]( google.com/amp/s/www.javaworld.com/article/2073723/core-java/…) Good unit tests for Customer will be of little use.
    – user949300
    Feb 19 '18 at 20:56
  • 1
    @user949300 ah, perhaps I shouldn’t have used the term “unit” tests. Your full set of automated tests will of course cover interactions between Customer and its dependencies and they will assist you in refactoring that class.
    – David Arno
    Feb 19 '18 at 22:17
0

Here is one big disadvantage: If I need to write code that handles the order or orders associated with a customer, I see that you are returning a list, so I will assume that there may be 0, 1 or 100 orders. And I will write code that can handle 0, 1 or 100 orders, and that will be more complicated than handling just 1. For example, I have to assume that things don't fit on one page of the screen and have to handle scrolling up and down. I have to sort the list this way and that way and write a UI for it. There's lots of extra work.

And then I get told there is only ever one order... I won't be happy.

You will save very little total work if you ever switch from one item to multiple ones, but you may waste a lot of work if you never make that switch.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.