Чистый код и гибридные объекты и зависть к функциям

10

Так что я недавно сделал несколько серьезных рефакторингов в своем коде. Одной из основных вещей, которые я пытался сделать, было разделение моих классов на объекты данных и рабочие объекты. Это было вдохновлено, среди прочего, этим разделом Чистого кода :

Гибриды

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

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

Недавно я просматривал код одного из моих рабочих объектов (который реализует шаблон посетителя ) и увидел следующее:

@Override
public void visit(MarketTrade trade) {
    this.data.handleTrade(trade);
    updateRun(trade);
}

private void updateRun(MarketTrade newTrade) {
    if(this.data.getLastAggressor() != newTrade.getAggressor()) {
        this.data.setRunLength(0);
        this.data.setLastAggressor(newTrade.getAggressor());
    }
    this.data.setRunLength(this.data.getRunLength() + newTrade.getLots());
}

Я сразу сказал себе: «Завидую особенностью! Эта логика должна быть в Dataклассе, особенно в handleTradeметоде, handleTradeи всегдаupdateRun должна происходить вместе». Но потом я подумал, что «класс данных - это просто структура данных, если я начну это делать, то это будет гибридный объект!»public

Что лучше и почему? Как вы решаете, что делать?

durron597
источник
2
Почему «данные» вообще должны быть структурой данных. Это имеет четкое поведение. Так что просто отключите все методы получения и установки, чтобы ни один объект не мог манипулировать внутренним состоянием.
Кормак Малхолл,

Ответы:

9

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

Например, мы можем утверждать, что трехмерный вектор является глупой записью. Однако это не должно мешать нам добавлять метод like add, который упрощает добавление векторов. Добавление поведения не превращает (не так глупо) запись в гибрид.

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

  • После обработки сделки последний агрессор может не обновиться.
  • Последний агрессор может быть обновлен, даже если никакой новой сделки не произошло.
  • Длина цикла может сохранить свое старое значение даже после обновления агрессора.
  • и т.п.

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

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

Недавно я написал ответ об инкапсуляции , в котором более подробно рассматривается , что такое инкапсуляция и как вы можете ее обеспечить.

Амон
источник
5

Тот факт, что handleTrade()и updateRun()всегда происходят вместе (а второй метод на самом деле находится на посетителе и вызывает несколько других методов для объекта данных), пахнет временной связью . Это означает, что вы должны вызывать методы в определенном порядке, и я предполагаю, что вызов методов не по порядку может что-то сломать в худшем случае или не даст значимого результата в лучшем случае. Фигово.

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

Старый код:

MyObject x = ...;
x.actionOne();
x.actionTwo();
String result = x.actionThree();

Новый код:

MyObject x = ...;
OneResult r1 = x.actionOne();
TwoResult r2 = r1.actionTwo();
String result = r2.actionThree();

Это имеет несколько преимуществ:

  • Он перемещает отдельные концерны в отдельные объекты ( SRP ).
  • Это устраняет временную связь: невозможно вызывать методы не по порядку, а сигнатуры методов предоставляют неявную документацию о том, как их вызывать. Вы когда-нибудь просматривали документацию, видели нужный объект и работали задом наперед? Я хочу объект Z. Но мне нужен Y, чтобы получить Z. Чтобы получить Y, мне нужен X. Ага! У меня есть буква W, необходимая для получения X. Цепите все это вместе, и теперь ваш W можно использовать для получения Z.
  • Такое разделение объектов с большей вероятностью сделает их неизменными, что имеет многочисленные преимущества, выходящие за рамки этого вопроса. Быстрый вывод состоит в том, что неизменяемые объекты имеют тенденцию приводить к более безопасному коду.

источник
Между этими двумя вызовами методов нет временной связи. Поменяйте местами их порядок и поведение не изменится.
durron597
1
Сначала я также думал о последовательном / временном соединении при чтении вопроса, но потом заметил, что updateRunметод был закрытым . Хороший совет - избегать последовательной связи, но она применяется только к разработке API / общедоступным интерфейсам, а не к деталям реализации. Похоже, реальный вопрос заключается в том, updateRunдолжен ли он быть в посетителе или в классе данных, и я не вижу, как этот ответ решает эту проблему.
Амон
Видимость не updateRunимеет значения, важна реализация, this.dataкоторой нет в вопросе и является объектом, управляемым объектом посетителя.
Во всяком случае, тот факт, что этот посетитель просто вызывает группу сеттеров и фактически ничего не обрабатывает, является причиной отсутствия временной связи. Вероятно, не имеет значения, в каком порядке вызывались сеттеры.
0

С моей точки зрения, класс должен содержать «значения для состояния (переменные-члены) и реализации поведения (функции-члены, методы)».

«Прискорбные гибридные структуры данных» появляются, если вы сделаете общедоступными переменные-члены состояния класса (или их методы получения / установки), которые не должны быть открытыми.

Поэтому я вижу, что нет необходимости иметь отдельные классы для объектов данных и рабочих объектов.

Вы должны быть в состоянии сохранить переменные-члены-состояния непубличными (ваш уровень базы данных должен уметь обрабатывать непубличные переменные-члены)

Зависть к объектам - это класс, который чрезмерно использует методы другого класса. Смотрите Code_smell . Наличие класса с методами и состоянием устранит это.

k3b
источник