Являются ли пустые абстрактные классы плохой практикой и почему? - PullRequest
10 голосов
/ 17 ноября 2009

У нас есть несколько пустых абстрактных классов в нашей кодовой базе. Я нахожу это уродливым. Но кроме этой очень глупой причины (уродства), я должен рефакторинг (в пустой интерфейс, например)?

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

Что ты думаешь?

РЕДАКТИРОВАТЬ:

1) Под "пустым абстрактным классом" я имею в виду что-то вроде:

public abstract class EmptyAbstractClass {}

2) Причина "пустоты": гибернация. Я не осваиваю эту структуру постоянства вообще. Я просто понимаю, что интерфейс не может быть сопоставлен с таблицей, и по этой технической причине класс был предпочтительнее интерфейса.

Ответы [ 11 ]

18 голосов
/ 17 ноября 2009

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

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

Другими словами, если это не сломано, не исправляйте это.

7 голосов
/ 17 ноября 2009

Вопрос, который нужно задать: «Что я хочу сделать с этим кодом, который я не могу или не могу сделать, потому что эти пустые абстрактные классы есть?» Если ответ «ничего», вы должны оставить их в покое. Если ответ «что-то», возможно, стоит удалить их, но если вы можете, сначала поговорите с людьми, которые их создали, просто чтобы убедиться, что для них нет какой-то тонкой цели. Например, возможно, ваш код использует отражение, чтобы найти все экземпляры определенного ABC и выполнить с ними особые действия, и в этом случае ваш рефакторинг может незаметно сломать код.

6 голосов
/ 17 ноября 2009

Это не обязательно более уродливо, чем альтернатива, которая может повторять код.

В идеальном мире вы сможете моделировать, используя только интерфейсы. например: Vehicel -> Автомобиль -> Понтиак.

Но может быть логика, которая одинакова для всех Vehicels, поэтому интерфейс не подходит. И у вас нет логики, специфичной для автомобилей. Но вам нужна автомобильная абстракция, потому что ваш TrafficLightController хочет различать автомобили и велосипеды.

В таком случае вам нужно сделать и абстрагировать Автомобиль.

Или вы можете создать интерфейс Vehicle, VehicleImpl реализует Vehicle, интерфейс Car расширяет Vehicle, интерфейс Pontiac реализует Car, а PontiacImpl реализует Pontiac расширяет VehicleImpl. Мне лично не нравится параллельная иерархия интерфейсов для предотвращения пустого абстрактного класса больше, чем пустого абстрактного класса.

Так что я думаю, это вопрос вкуса.

Одна оговорка; если вы используете много прокси-классов, таких как Spring и некоторые тестовые среды, иерархия параллельного интерфейса вполне может привести к менее неожиданным ошибкам.

5 голосов
/ 18 ноября 2009

Мне кажется, что это результат создания иерархии объектов, которая в итоге не имеет какой-либо общей функциональности на самых верхних уровнях. Я подозреваю, что непосредственные подклассы являются абстрактными сами по себе или, по крайней мере, имеют свои подклассы. Другая вероятность состоит в том, что в вашем коде много экземпляров instanceof, разбросанных по нему.

Пустой верхний уровень сам по себе не так уж и сложен, но я бы проверил, чтобы на самом деле не было никакой общей функциональности. Предполагая, что это действительно существует, я бы посмотрел на то, как подтянуть общие черты в родительских классах. Я бы определенно следил за instanceof и серьезно задумывался о его рефакторинге (примеры см. В разделе «Рефакторинг для Patterns (Kerievsky)»).

2 голосов
/ 17 ноября 2009

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

  1. Подождите, пока вам не понадобится расширить другой класс для реальной необходимости, и вас раздражает абстрактный класс, чтобы заменить его интерфейсом.
  2. Не ждите и исправьте дизайн.

В этой конкретной ситуации, правда, что настоящий дизайн на самом деле пока не повредит, так что вы можете с этим жить. Тем не менее, я думаю, что замена этих абстрактных классов довольно проста в рефакторинге (сделайте их интерфейсами и замените extends на implements, где вы получите ошибки компиляции), и я действительно не вижу, что может сломаться, поэтому я бы пошел на это.

Лично, и люди, конечно, могут не согласиться, я не люблю быть слишком оборонительным. С такими правилами, как , если это не сломано, не исправляйте это , вы никогда не будете рефакторировать что-либо («мои тесты пройдены, зачем мне рефакторинг?»). Замораживание кода - определенно не правильное решение, агрессивное тестирование - правильное решение .

2 голосов
/ 17 ноября 2009

Задайте себе этот вопрос: если в результате вашего рефакторинга что-то сломается на производстве и ваша постоянная занятость зависит от того, насколько хорошо вы оправдываете свое решение потратить время на исправление чего-то, что на самом деле не сломалось, что вы скажете?

«Это было уродливо и эстетически оскорбительно для меня» - это не тот ответ, на который я хотел бы поставить свою работу.

На этом этапе я говорю: будь осторожен и живи с Гадким.

1 голос
/ 17 ноября 2009

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

Судя по всему, решение о разработке "пустого абстрактного класса" было сделано таким образом, чтобы оно не позволяло реализующему классу расширяться из других классов.

Если бы это был я, я бы его отпустил, иначе он может сломаться. Лучше всего связаться с разработчиками оригинала и попросить аргументации (и добавить их в качестве комментариев в абстрактный класс для вашего и будущего удобства).

1 голос
/ 17 ноября 2009

Согласно теории объектно-ориентированного программирования основной целью наследования является полиморфизм, повторное использование кода и инкапсуляция. Пустой абстрактный класс (и когда я говорю это, я имею в виду действительно пустой, без конструкторов, без методов, без свойств. Ничего!) Не достигает ни одной из трех целей, на которые рассчитывает этот метод программирования. Это эквивалентно if(true){...}. изменение его на интерфейс на самом деле не делает его лучше.

Если вы хотите провести рефакторинг кода, я бы посоветовал вам думать в направлении, противоположном тому, о котором вы думаете, и я имею в виду следующее: попытаться абстрагировать свойства, методы и конструкторы от всех классов, которые имеют абстрактный родитель класс.

Это тяжелая работа с небольшим вознаграждением в краткосрочной перспективе, но она значительно повышает удобство сопровождения кода, поскольку изменение логики ядра должно быть выполнено только один раз. Я подозреваю, что причина использования этих пустых абстрактных классов состоит в том, чтобы идентифицировать группу классов, которые должны иметь что-то общее, в противном случае, в чем будет разница между Object и абстрактным классом

0 голосов
/ 27 июля 2011

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

Что бы я определенно сделал: напишите комментарий о том, почему этот класс существует в файле. Одна большая причина для чистого и «красивого» кода - не заставлять других разработчиков думать. В этом может помочь комментарий, даже если код не такой красивый, как мог бы.

0 голосов
/ 17 ноября 2009

Очевидные вопросы: почему это было положено туда? и как это используется?

Я предполагаю, что либо (а) это пережиток какого-то фальстарта. В раннем черновике были некоторые данные или функции в абстрактном классе, но когда программист работал, все они были перемещены в другое место, пока не осталось ничего, кроме пустой оболочки. Но более вероятно (б) В какой-то момент есть код, который говорит "if (x instanceof ...". Я думаю, что это плохой дизайн, в основном, использование членства в классе в качестве флага. Если вам нужен флаг, создайте флаг не притворяйтесь, что вы «более объектно-ориентированы», создав класс или интерфейс, а затем используя его в качестве флага. Это может соответствовать определению из учебника о лучшем кодировании, но на самом деле код только запутывает.

+ 1 Монаху за то, что он указал, что пустой интерфейс не лучше, чем пустой абстрактный класс. Да, да, я знаю, что Sun сделала это самостоятельно с помощью Cloneable, но я думаю, что это было неудачное решение проблемы.

...