When I read various tutorials, and our favorite StackOverflow, I often see similar code :

namespace MvcApplication2.Models { public class Category { public int ID { get; set; } public string Name { get; set; } } public class Product { public ICollection<int> CategoryID { get; set; } public Product() { CategoryID = new List<int>(); } } }

Explain why the CategoryID property is declared as an ICollection interface if it is explicitly initialized in the constructor using the List ?

What does the designer try to avoid with this approach?

I understand if some kind of dependency was introduced into the Product class through its constructor. But this is not.

What is the conceptual moment I missed?

    2 answers 2

    The value sticks out and the client code does not know that there is a List . That is, at the behest of the left heel of the architect, in the new version of the List library can be replaced by LinkedList and no one will suffer. This is an abstraction over the implementation of the collection.

    • four
      @adamshakhabov, if you set the List and the client code calls the Product.CategoryId.ForEach() method, then changing the collection type will break the code. And if an ICollection set, there will be no such code. The only problem with ICollection can be if an Array assigned there. - Vlad
    • 2
      @adamshakhabov Product is a public type, i.e. it can be accessed from other assemblies. Inside the Product is at least the public CategoryID property. Than it is initialized inside the type - these are the internal difficulties of the Product. You can tighten this assembly in the form of dll. That is, you do not even have to rebuild your project (thanks to abstraction and mandatory dynamic linking in .NET). - free_ze
    • one
      @adamshakhabov, 1) yes, you prevent the use of methods that are specific to certain collections in client code. In particular, ForEach . 2) Nothing prevents. But, for example, you realized at some point that you need to track changes in this collection. You can replace it with ObservableCollection and subscribe to its events. At the same time, nothing will change for the calling code. - Vlad
    • one
      In fact, this type does not encapsulate anything;) Here all the data is open, no behavior. And if in the collection (which is mutable), a kind person will record an array that will have a lot of fun happening at runtime;) - Sergey Teplyakov
    • one
      @SergeyTeplyakov We are here to theorize ;-) - free_ze

    Here the author, in theory, tries to follow the principle of least knowledge and does not expose the details of implementation.

    Perhaps this code is old or all components are not shown, but in this implementation the design is not good.

    1. CategoryID property is mutable. The client can write there a null followed by a raking of a NullReferenceException .

    2. The property only tries to hide implementation details, but does it poorly. Everything strongly depends on the context, but there are at least two ways to make this design more rigid on the one hand and simpler on the other.

    First, you can make the type immutable and get the collection in the constructor. In this case, the CategoryID property instead of the IReadOnlyCollection type can become an IReadOnlyCollection or an IReadOnlyList .

    Secondly, if a class cannot be made immutable, then it makes sense to add the AddCategory method and still make the CategoryID property type IReadOnlyCollection/IReadOnlyList .

    It is difficult to speak here without context, but I am always amazed by such data objects in namespaces named Model . The model in the namespace name tells me that the whole essence of the application, its domain objects, with the behavior and all kinds of bells and whistles will be hidden here. And when I see simple data objects in such a namespace, I have some mismatch of expectations with reality.

    In other words, if there is a desire to create models, then it makes sense to hide the insides fully, rather than removing the "real type of list." Then it will be possible to add higher-level behavior (some sort of filtering logic by categories and something else) without breaking existing customers.

    And now a little on the topic:

    The interfaces of the collections in BCL are a little crazy in the sense that it is now very difficult to say what they mean. This is especially true for the ICollection type: what is this collection? Is it mutable? It seems to be yes, there is a method Add . But the trouble is, arrays also implement ICollection<T> , whose Add method throws an exception. Yes, the IsReadOnly property is IsReadOnly , but are all the clients checking it for sure?

    Well, of course, the collections have Contains and Remove methods, but the first one is O (N), which is almost always bad, and the second one also does not work for all collections.

    It turns out that this interface is often used as an IEnumerable + Count , but in this case IReadOnlyXXX views are also better IReadOnlyXXX .

    As a conclusion: you need to understand what and from whom you are hiding and whether you are hiding anything at all. If this code is used in applications, then there are two options: use a specific collection, if the class is a data repository or hide the collection fully and expose an IReadonlyXXX representation with specialized Add methods.

    • @downvoter: it will be interesting to discuss what was not pleasant in the answer;) - Sergey Teplyakov Nov.
    • such a call will not work and no one will see him. It is better to write on the meta more likely that they will see and can answer. I put a minus, because in my opinion, this answer is a stream of consciousness that does not intersect with the question. - Grundy Nov.
    • one
      @PavelMayorov: if it is a simple intermediary between two layers, then there is nothing to be embarrassed and use specific types of collections. After all, the encapsulation here does not smell. It's like a triple burger with diet coke. The main problem is not in the type of collection, but in the openness of the data :) - Sergey Teplyakov
    • one
      Well, I call them DTO then. Data etc. The word “model”, nevertheless, implies either the process of modeling some reality (which is not present), or the domain, or something else. Apparently the word "model" here reflects M in the MVP, MVC patterns. There, these classes are also usually associated with the subject area. We can say that this area is so simple that it does not need a full-fledged OOP. But then, as I already wrote, there is nothing to bother with interfaces. - Sergey Teplyakov Nov.
    • one
      Pro interfaces agree. Thank you for the clarification. - Vlad