Вопрос

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

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, чтобы очистить его.

Мои вопросы таковы:

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

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

Это было полезно?

Решение

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

Я бы реорганизовал в Options - передан на завод, и Results, вернувшийся из analyseText() способ.

Другие советы

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

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

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

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

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

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

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

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top