2

I have a class, which looks like this:

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

    public FirstName FirstName { get; set; }
    public LastName LastName { get; set; }
    public Province Province { get; set; }
    public IEnumerable<Order> Orders 
    {
        get { foreach (var order in _orders) yield return order; }
    }

    internal void AddOrder(Order order)
    {
        _orders.Add(order);
    }
    //Planning to add an AddRange method to add a collection of Orders.
}  

Someone suggested to me recently that I should be using a Set instead of a list as I do not need the index benefits of a list as it is exposed as an IEnumberable.

If I change this to a set, then how do I implement equality? I believe I have a few options:

1) Leave it as a list.

2) Do nothing - then I believe orders are unique based on referential equality. Is there any risk doing this? Everywhere I read tells me that you must override Hashcode and equals if you are using a Hashset. Here is an example of a Set, where the default Object.hashcode and object.equals appears to be used with a HashSet: https://github.com/nhibernate/nhibernate-core/blob/master/src/NHibernate.DomainModel/Northwind/Entities/Order.cs

3) Override .equals and .hashcode in the Order class so that Orders are equal if they have the same ID. This link suggests that you should not do this: https://www.youtube.com/watch?v=xRCOKKUSp9s

4) Create an Entity base class similar to this: https://github.com/VaughnVernon/IDDD_Samples_NET/blob/master/iddd_common/Domain.Model/Entity.cs. The youtube video in point three appears to advise against this.

5) Implement an IEqualityComparer. The research I have done suggests this is a bad idea because: 1) I will have to inject/pass a comparer into the entity and 2) The code for establising whether two orders are equal is in a different class to Order making the domain model anemic.

I am trying to follow the principle of least astonishment and find myself going round in circles sometimes trying to achieve this. A lot of the links above are several years old. Is there a standard way to approach this?

  • 2
    Someone suggested to me recently that I should be using a Set instead of a list as I do not need the index benefits and? Did that someone also say why is that so terrible? KISS!!! If using Set is causing you too much overhead, go with Lists. Do refactor later if you deem it necessary – Laiv Apr 14 '18 at 11:09
  • @Laiv, thanks +1 for KISS. That is what I thought. They just said that I do not need the index benefits of a list and also a set is more efficient. – w0051977 Apr 14 '18 at 11:16
  • 2
    And, as always, don't fall in the trap of pre-optimising. Your time is more expensive than CPU cycles. – MetaFight Apr 14 '18 at 11:18
  • 1
    @w0051977, sorry, I was just trying to draw attention to the fact that, by switching to a Set, you'd be sacrificing other List features that you might actually need (eg, item order). – MetaFight Apr 14 '18 at 11:25
  • 1
    Take a look to this post. Note that the author is pointing to some performance issues that could be mitigated by HashSets. However, you should not be worried about them till they become a real concern. Solve this sort of problems as they apear. – Laiv Apr 14 '18 at 12:47
3

Allow me to approach the answer from different angles.

Premature optimization

You have been told that there're other collections whom, apparently, are more suitable your case. It's probably true, but I don't find the given arguments to be enough to make me believe that List is utterly out of place.

Ask yourself whether HashSet solves a real and known issue of the current implementation. Ask as well if these unused List's capabilities are counterproductive. Finally, evaluate whether the effort of changing the implementation makes a big improvement. But don't spend too much time.

Wasting too much time answering these questions (or refactoring prematurely) instead of solving the problem, diminish the benefits of one or another implementation. Solving problems you don't have is premature optimization.

As @MetaFight says, your time is more valuable than CPU cycles.

Equality

What Equality means should be up to you to decide. The ubiquitous language has much to say here. If not, ask the domain experts when two Orders are equals and when they are exactly the same.

Checking these conditions should not depend on the collection type you choose. One type can help you out to achieve it easier than other, but not at the cost of condition the design.

Take the following in mind. Equals and Same are different things.

Two references could be:

  • pointing to the same instance (in memory)

  • pointing to different instances whose data are equals. What causes both to have the same hashcode.

  • pointing to different instances, with different data and still have the same hashcode (collision). Rare, but possible.

  • pointing to different instances, with different data and they to be considered equals by the domain, but not by the collection.

Delegating the control of Equality and Same to the collection might not be enough for you to express the business rules.

For example, if having the same Order twice in the same collection is not allowed but having two equals is, would be good to make this constraint explicit with your own classes and functions3, so that other developers can "read" what equal/same mean and when these characteristics are important.

The reason for all of this is simple. Expressiveness.2

Implementing code to express the differences between "same" and "equals" 1 follows the principle of least astonishment since it's not required for us to be familiar with the SDK.

Don't make us look in the SDK for us to know what are you trying to do.

Testing

Short story long. If we delegate the responsibility to the collection, when it comes to testing, we would be testing the collection instead of the business rules.

KISS

One collection could be more suitable than others. But, if List meet the needs and it's the simplest solution, then go with List. What collection to use is technical detail. Ideally, the domain model is agnostic to this sort of details. If it's, it's easier for you to change the implementation from one collection to another.


1: Or any other constraint of the domain

2: I suggest you to read @VoiceOfUnreason's answer for more insights regarding this subject

3: or other elements of the domain

  • +1 for KISS principle. Say I was to introduce a hashset in the future. Is it ever acceptable not to rely on default equality? Everywhere I read says you MUST not do this. However, if you are happy to rely on default equality then I don't see what the issue is. – w0051977 Apr 14 '18 at 18:04
  • If It meet the needs, then it's ok. What matters to me is to be aware of the consequences of the choices we do. So, if we ever are proved to be wrong, we know why and how to address it. – Laiv Apr 14 '18 at 18:15
  • If it meet the needs, then it's ok. What matters to me is to be aware of the consequences of the choices we do. So, if we ever are proved to be wrong, we know why and how to address it. Again, the principle of the least astonishment. – Laiv Apr 14 '18 at 18:15
  • Finally, what dot you mean by "make this constraint explicit with code". Are you saying that the repository should return the distinct orders? – w0051977 Apr 14 '18 at 18:30
  • I do mean to implement the control of Equality, duplicated orders, etc, with your own code (classes, functions, etc). Where to put that code depends on the case. Some may go in repositories, others in aggregates. Who do you think is responsible of each decision? – Laiv Apr 14 '18 at 18:47
4

It looks to me as though you are getting tangled because you are mixing two different abstraction layers here.

List and HashMap are data structures, they are different ways of arranging collections in memory (with slightly different invariants). Your domain models normally shouldn't care about this level of detail.

Compare with

private readonly OrderHistory _orders = new OrderHistory();

The model for customer doesn't need to know the details of how orders are represented in memory, it just needs to know the semantics of accessing them.

Your value types are typically where you start worrying about specific in memory representations. A good exercise to help emphasize this is the probability kata. They bridge the gap between the semantics of the model and some data structure that supports those semantics.

In other words, the OrderHistory abstraction should be insulating the model from the decision you have made about which in memory representation to use. See Parnas on decomposing systems into modules.

Now, for equality, you need to understand the semantics in your model.

That is, if lhs === rhs, what guarantees are implied? What two things being equal really tells you is that, for some family of functions f, f(lhs) == f(rhs). But the family of functions there depends on your domain model.

For instance - do two different histories have to enumerate their entries in the same order? Do they have to point to the same objects (same locations in memory), or just objects that are equivalent? (If orders are "value objects", then you probably only need them to be equivalent.)

The semantic constraints on "equality" in your model might be more permissive than those provided by your collection library.

Because the details are encapsulated, you have an opportunity to perform any necessary conversions. For instance suppose you already have some function isSameHistory(IList<Order> lhs, IList<Order> rhs) and you've testing for equality like

boolean isSameCustomer(Customer lhs, Customer rhs) {
    return isSameHistory(lhs._orders, rhs._orders);
}

and you later need to re-implement _orders as an array? There's a pretty quick refactoring available

boolean isSameCustomer(Customer lhs, Customer rhs) {
    return isSameHistory(lhs._orders.toList(), rhs._orders.toList());
}

Meaning that switching from one representation to another in a functionally equivalent way should be straight forward. You might have to do some refactoring work in advance to make this change easy.

  • Great answer. I would dare to say also that if we model the constraints explicitly rather than laying on the technical details of the data structure, only take us to implement the unit test of these constraints for us to change implementations with guarantee. Otherwise, we would be just testing the proper functioning of the data structure (unecessary) and bound to whatever the future holds for it. – Laiv Apr 14 '18 at 16:57
  • 1
    Thanks for referencing the Parnas paper, which, IMO, is far better and far clearer than the overrated SRP. – user949300 Apr 14 '18 at 19:18
  • Could you clarify what you mean by: "If orders are "value objects", then you probably only need them to be the same.". When you say "the same" I assume you mean referential equality. Why is this specific to value objects? +1. – w0051977 Apr 15 '18 at 12:04
  • That part of the answer wasn't expressed well, rephrased. – VoiceOfUnreason Apr 15 '18 at 14:05

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.