"Имеет" против "Есть" - код пахнет для принятия решения

Question

Я написал это вчера, в классе Foo, наследующем от Bar:

public override void AddItem(double a, int b)
{
    //Code smell?
    throw new NotImplementedException("This method not usable for Foo items");
}

Интересно, будет ли это возможным указанием на то, что я должен использовать бар, а не наследовать от него.

Какие еще «запахи кода» могли бы помочь выбрать между наследованием и композицией?

РЕДАКТИРОВАТЬ Я должен добавить, что это фрагмент, есть другие методы, которые являются общими , я просто не хотел вдаваться в подробности. Я должен проанализировать последствия перехода на композицию и подумать, могут ли быть другие «запахи кода», которые могли бы помочь изменить баланс.

Answer 1

Пример, который вы привели выше, явно пахнет кодом. Метод AddItem является поведением базового класса Bar. Если Foo не поддерживает поведение AddItem, оно не должно наследоваться от Bar.

Давайте подумаем о более реалистичном (C ++) примере. Допустим, у вас был следующий класс:

class Animal
{
    void Breathe() const=0;
}

class Dog : public Animal
{
    // Code smell
    void Breathe() { throw new NotSupportedException(); }
}

Базовый абстрактный класс Animal предоставляет чисто виртуальный метод Breathe(), потому что животное должно дышать, чтобы выжить. Если он не дышит, то по определению это не животное.

Создав новый класс Dog, который наследуется от Animal, но не поддерживает поведение Breathe(), вы нарушаете контракт, предусмотренный классом Animal. Бедная собака не выживет!

Простое правило публичного наследования заключается в том, что вы должны делать это только в том случае, если объект производного класса действительно является «объектом» базового класса.

В вашем конкретном примере:

• Foo не поддерживает поведение AddItem(), предусмотренное контрактом Bar.
• Следовательно, по определению, Foo не является "Bar и не должно наследоваться от него.

Answer 2

Нет! Квадрат может быть прямоугольником, но Квадратный объект определенно не Прямоугольный объект. Зачем? Поскольку поведение квадратного объекта не в соответствии с поведением Прямоугольный объект. Поведенческий, Квадрат не прямоугольник! И это Поведение, что программное обеспечение действительно все о.

С Принцип подстановки Лискова в Object Mentor.

Answer 3

Да, то, что вы должны «не реализовывать» методы, является признаком того, что, возможно, вам не следует использовать отношения «есть». Ваши Foos, кажется, действительно не Бары.

Но начните с размышлений о своих Foos и барах. Такое бары Foos? Можете ли вы нарисовать наборы и подмножества на бумаге, и будет ли каждый Foo (то есть каждый член класса Foo) также быть Bar (то есть членом класса Bar)? Если нет, вы, вероятно, не хотите использовать наследование.

Еще один запах кода, который указывает на то, что Foos на самом деле не Bars и что Foo не должен наследовать Bar, состоит в том, что вы не можете использовать полиморфизм. Допустим, у вас есть метод, который принимает Bar в качестве аргумента, но он не сможет обработать Foo. (Возможно, потому что он вызывает метод AddItem в своем аргументе!) Вам придется добавить проверку или обработать исключение NotImplementedException, что сделает код сложным и трудным для понимания. (И пахнущий!)

Answer 4

Так почему бы вам наследовать от Bar, если Foo без расширений не ведет себя как Bar? Если подумать, я бы даже не объявил метод типа AddItem виртуальным в базовом классе.

Answer 5

Хотя приведенный выше пример, вероятно, указывает, что что-то идет не так, само NotImplementedException не всегда неверно. Это все о контракте суперкласса и о подклассе, реализующем этот контракт. Если у вашего суперкласса есть такой метод

// This method is optional and may be not supported
// If not supported it should throw NotImplementedException 
// To find out if it is supported or not, use isAddItemSupported()
public void AddItem(double a, int b){...}

Тогда, не поддерживая этот метод, все еще в порядке с контрактом. Если это не поддерживается, вам, вероятно, следует отключить соответствующее действие в пользовательском интерфейсе. Так что, если вы согласны с таким контрактом, то подкласс реализует его правильно.

Другой вариант, когда клиент явно заявляет, что он не использует все методы класса и никогда не будет использовать. Как это

// This method never modifies orders, it just iterates over 
// them and gets elements by index.
// We decided to be not very bureaucratic and not define ReadonlyList interface,
// and this is closed-source 3rd party library so you can not modify it yourself.
// ha-ha-ha
public void saveOrders(List<Order> orders){...}

Тогда можно перейти к реализации List, которая не поддерживает добавление, удаление и другие мутаторы. Просто документируйте это.

// Use with care - this implementation does not implement entire List contract
// It does not support methods that modify the content of the list.
public ReadonlyListImpl implements List{...}

Хотя ваш код для определения всего вашего контракта хорош, так как он позволяет вашему компилятору проверять, нарушаете ли вы контракт, иногда это нецелесообразно, и вам приходится прибегать к плохо определенным контрактам, таким как комментарии.

Короче говоря, возникает вопрос: действительно ли вы можете безопасно использовать свой подкласс в качестве суперкласса, принимая во внимание, что суперкласс определяется его контрактом, который является не только кодом.

Answer 6

Конечно, существует прямое обратное, если ваш Foo реализует много интерфейса, который просто перенаправляет сообщения на принадлежащий ему Бар, это может указывать на то, что он должен быть Бар. Впрочем, только запахи не всегда правильные.

Answer 7

Композиция предпочтительнее для наследования, так как это уменьшает сложность. Лучше было бы использовать инъекцию в конструктор и сохранить ссылку на Bar в вашем классе Foo.

Answer 8

.NET Framework имеет такие примеры, особенно в пространстве имен System.IO - некоторые читатели не реализуют все свои свойства / методы базового класса и выдают исключения, если вы пытаетесь их использовать.

например. Stream имеет свойство position, но некоторые потоки не поддерживают это.