Секретный рукопожатие анти-шаблон - PullRequest
8 голосов
/ 27 апреля 2009

Я только что натолкнулся на образец, который видел раньше, и хотел узнать мнение по этому поводу. В рассматриваемом коде используется такой интерфейс:

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, но я вижу большую разницу, потому что это платформа, использующая пользовательский код для реализации этих интерфейсов, а не сервис, который может иметь несколько реализаций. Я полагаю, что все, что связано с включением «закрытого» метода, который должен быть вызван, является оправданным, поскольку другого разумного способа сделать это не существует. В некоторых случаях, например, в JDBC, это является следствием утечки абстракции, но в двух частях кода, о которых я думаю, это довольно очевидно, что программисты поспешно добавляют интерфейс к классу кода спагетти, чтобы очистить его.

Мои вопросы:

  1. Все согласны с тем, что это плохо спроектированный интерфейс?
  2. Это описанный анти-паттерн?
  3. Этот тип инициализации когда-либо принадлежит интерфейсу?
  4. Мне это кажется неправильным только потому, что я предпочитаю функциональный стиль и неизменность?

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

Ответы [ 5 ]

17 голосов
/ 27 апреля 2009

Да, это анти-шаблон: Последовательное соединение .

Я бы преобразовал в Options - передал на фабрику, а Results, вернул метод analyseText().

3 голосов
/ 27 апреля 2009

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

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

Джошуа Блох на самом деле имеет отличную презентацию (36м16 и 40м30) по разработке API, и он рассматривает это как одну из характеристик плохо спроектированного API.

3 голосов
/ 27 апреля 2009
  1. Я ожидаю, что AnalyzerFactory получит необходимые параметры и выполнит саму конструкцию; в противном случае, что именно он делает?
  2. Не уверен, что у него есть имя, но похоже, что оно должно:)
  3. Да, иногда удобно (и правильный уровень абстракции) иметь сеттеры в вашем интерфейсе и ожидать, что классы их вызовут. Я бы предположил, что для требуется обширная документация по этому факту.
  4. Не совсем, нет. Предпочтение неизменности - это, конечно, хорошая вещь, и дизайн, основанный на установщиках / bean-компонентах, иногда может быть и «правильным» выбором, но в данном примере это слишком далеко.
0 голосов
/ 24 августа 2017

Одно из возможных решений - использовать Fluent chaning. Это позволяет избежать класса, содержащего методы, которые необходимо вызывать в определенном порядке. Это очень похоже на шаблон построителя, который гарантирует, что вы не читаете объекты, которые все еще находятся в процессе заполнения.

0 голосов
/ 27 апреля 2009

Я не вижу здесь ничего плохого. setText() готовит сцену; после этого у вас есть один или несколько звонков на getOccurances(). Поскольку setText() очень дорого, я не могу придумать другого способа сделать это.

getOccurances(text, query) исправит «секретное рукопожатие» с огромной производительностью. Вы можете попытаться кэшировать текст в getOccurances() и обновлять свои внутренние кэши только тогда, когда текст меняется, но это начинает все больше походить на жертву некоему принципу ОО. Если правило не имеет смысла, не применяйте его. У разработчиков программного обеспечения есть разум.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...