Здесь автор, по идее, старается следовать принципу наименьшего знания и не выставлять наружу детали реализации.
Возможно данный код старый или не показаны все составляющие, но в данной реализации дизайн не хорош.
Свойство CategoryID
мутабельное.
Клиент может записать туда null
с последующим огребанием NullReferenceException
.
Свойство только старается спрятать детали реализации, но делает это плохо.
Все сильно зависит от контекста, но существует как минимум два способа сделать этот дизайн более жестким с одной стороны и более простым с другой.
Во-первых, можно сделать тип неизменяемым и получать коллекцию в конструкторе. В этом случае, свойство CategoryID
вместо типа ICollection
может стать IReadOnlyCollection
или IReadOnlyList
.
Во-вторых, если класс нельзя сдлать иммутабельным, то есть смысл добавить метод AddCategory
и все же сделать свойство CategoryID
типом IReadOnlyCollection/IReadOnlyList
.
Тут сложно говорить без контекста, но меня всегда поражают подобные объекты-данные в пространствах имен с именем Model
. Модель в имени пространства имен говорит мне о том, что здесь будет спрятана вся суть приложения, ее доменные объекты, с поведением и всякими наворотами. А когда я вижу в таком пространстве имен простые объекты-данные, у меня происходит некоторое несовпадение ожиданий с реальностью.
Другими словами, если есть желание создавать модели, то есть смысл прятать внутренности полноценно, а не убирая "реальный тип списка". Тогда можно будет добавлять более высокоуровневое поведение (какую-нибудь логику фильтрации по категориям и чего-нить еще) не ломая существующих клиентов.
А теперь немного по теме:
Интерфейсы коллекций в BCL немного сумасшедшие в том плане, что сейчас уже очень сложно сказать, что они означают. Это особенно относится к типу ICollection
: что это за коллекция? Является ли она изменяемой? Вроде бы да, там же есть метод Add
. Но вот беда, массивы тоже реализуют ICollection<T>
, метод Add
которых бросает исключение. Да, там есть свойство IsReadOnly
, но точно ли все клиенты его проверят?
Ну, конечно, у коллекций есть методы Contains
и Remove
, но первый дает сложность O(N), что почти всегда плохо, а второй также не работает для всех коллекций.
Вот и получается, что этот интерфейс зачастую используется в качестве такого себе IEnumerable + Count
, но и в этом случае лучше подойдут IReadOnlyXXX
представления.
В качестве заключения: нужно понимать что и от кого вы прячите и прячите ли вы вообще что-либо. Если данный код используется в приложения, то есть два варианта: использовать конкретную коллекцию, если класс является хранилищем данных или же прятать коллекцию полноценно и выставлять IReadonlyXXX
представление со специализированными методами Add
.