Секретный Анти-Паттерн Рукопожатия
-
16-09-2019 - |
Вопрос
Я только что наткнулся на шаблон, который видел раньше, и хотел узнать мнение о нем.Рассматриваемый код включает в себя интерфейс, подобный этому:
public interface MyCrazyAnalyzer {
public void setOptions(AnalyzerOptions options);
public void setText(String text);
public void initialize();
public int getOccurances(String query);
}
И ожидаемое использование выглядит следующим образом:
MyCrazyAnalyzer crazy = AnalyzerFactory.getAnalyzer();
crazy.setOptions(true);
crazy.initialize();
Map<String, Integer> results = new HashMap<String, Integer>();
for(String item : items) {
crazy.setText(item);
results.put(item, crazy.getOccurances);
}
Для чего-то из этого есть причины.setText(...) и getOccurances(...) существуют потому, что есть несколько запросов, которые вы, возможно, захотите выполнить после выполнения одного и того же дорогостоящего анализа данных, но это может быть преобразовано в результирующий класс.
Почему я думаю, что это так плохо:реализация сохраняет состояние способом, который четко не указан интерфейсом.Я также видел нечто подобное, связанное с интерфейсом, который требовал вызова "prepareResult", затем "GetResult".Теперь я могу придумать хорошо продуманный код, который использует некоторые из этих функций.Интерфейс Hadoop Mapper расширяет возможности JobConfigurable и Closeable, но я вижу большую разницу, потому что это фреймворк, который использует пользовательский код, реализующий эти интерфейсы, по сравнению с сервисом, который может иметь несколько реализаций.Я полагаю, что все, что связано с включением метода "close", который должен быть вызван, оправдано, поскольку другого разумного способа сделать это нет.В некоторых случаях, таких как JDBC, это следствие непрочной абстракции, но в двух фрагментах кода, о которых я думаю, это довольно явно следствие того, что программисты поспешно добавили интерфейс к классу кода spaghetti, чтобы очистить его.
Мои вопросы таковы:
- Все согласны с тем, что это плохо спроектированный интерфейс?
- Является ли это описанным анти-паттерном?
- Принадлежит ли этот вид инициализации когда-либо интерфейсу?
- Кажется ли это мне неправильным только потому, что я предпочитаю функциональный стиль и неизменность?
Если это достаточно распространено, чтобы заслуживать названия, я предлагаю антишаблон "Секретное рукопожатие" для интерфейса, который заставляет вас вызывать несколько методов в определенном порядке, когда интерфейс по своей сути не отслеживает состояние (например, коллекция).
Решение
Да, это анти-паттерн: Последовательное соединение.
Я бы реорганизовал в Options
- передан на завод, и Results
, вернувшийся из analyseText()
способ.
Другие советы
- Я бы ожидал увидеть, что AnalyzerFactory получит необходимые параметры и выполнит саму конструкцию;в противном случае, что именно он делает?
- Не уверен, есть ли у него название, но, похоже, так и должно быть :)
- Да, иногда удобно (и с правильным уровнем абстракции) иметь установщики в вашем интерфейсе и ожидать, что классы будут вызывать их.Я бы посоветовал сделать это требует обширная документация этого факта.
- Не совсем, нет.Предпочтение неизменяемости, безусловно, хорошо, и дизайн на основе setter / bean иногда тоже может быть "правильным" выбором, но ваш приведенный пример заходит слишком далеко.
Я не уверен, является ли это описанным антишаблоном, но я полностью согласен, что это плохо спроектированный интерфейс.Это оставляет слишком много возможностей для ошибок и нарушает по крайней мере один ключевой принцип:сделайте так, чтобы вашим API было трудно злоупотреблять.
Помимо неправильного использования, этот API также может привести к трудноотлаживаемым ошибкам, если несколько потоков используют один и тот же экземпляр.
Джошуа Блох на самом деле обладает отличным презентация (36m16s и 40m30s) о дизайне API, и он рассматривает это как одну из характеристик плохо спроектированного API.
Я не вижу здесь ничего плохого. setText()
подготавливает сцену;после этого у вас есть один или несколько звонков в getOccurances()
.С тех пор как setText()
это так дорого, что я не могу придумать никакого другого способа сделать это.
getOccurances(text, query)
исправил бы "секретное рукопожатие" с огромными затратами на производительность.Вы могли бы попробовать кэшировать текст в getOccurances()
и обновляйте свои внутренние кэши только тогда, когда текст меняется, но это начинает все больше и больше походить на принесение в жертву какому-то принципу OO.Если правило не имеет смысла, то не применяйте его.У разработчиков программного обеспечения есть мозг не просто так.
Одно из возможных решений - использовать Fluent chaning.Это позволяет избежать класса, содержащего методы, которые должны вызываться в определенном порядке.Это очень похоже на шаблон builder, который гарантирует, что вы не будете читать объекты, которые все еще находятся в процессе заполнения.