Лучше иметь 2 метода с ясным значением или только 1 метод двойного использования?

30

Чтобы упростить интерфейс, лучше просто не иметь getBalance()метод? Переход 0к charge(float c);тому же результату даст:

public class Client {
    private float bal;
    float getBalance() { return bal; }
    float charge(float c) {
        bal -= c;
        return bal;
    }
}

Может быть, сделать заметку в javadoc? Или просто оставьте это пользователю класса, чтобы выяснить, как получить баланс?

Дэвид
источник
26
Я щедро предполагаю, что это пример, иллюстративный код и что вы не используете математические вычисления с плавающей запятой для хранения денежных значений. В противном случае мне придется все проповедовать. :-)
corsiKa
1
@corsiKa нет. Это всего лишь пример. Но да, я бы использовал float для представления денег в реальном классе ... Спасибо, что напомнили мне об опасностях чисел с плавающей запятой.
Дэвид
10
Некоторые языки различают мутаторы и аксессоры с хорошим эффектом. Было бы ужасно раздражать, если не удалось получить баланс экземпляра, который доступен только как константа!
JDługosz

Ответы:

90

Вы, кажется, предполагаете, что сложность интерфейса измеряется количеством элементов, которые он имеет (методы, в данном случае). Многие утверждают, что необходимость помнить, что chargeметод может использоваться для возврата баланса, Clientдобавляет гораздо больше сложности, чем наличие дополнительного элемента getBalanceметода. Делать вещи более явными гораздо проще, особенно до такой степени, что это не оставляет двусмысленности, независимо от большего количества элементов в интерфейсе.

Кроме того, вызов charge(0)нарушает принцип наименьшего удивления , также известный как показатель WTFs в минуту (из «Чистого кода», изображение ниже), что затрудняет работу новых членов команды (или текущих, через некоторое время вдали от кода) до они понимают, что звонок фактически используется для получения баланса. Подумайте, как отреагируют другие читатели:

введите описание изображения здесь

Кроме того, сигнатура chargeметода идет вразрез с рекомендациями делать одно и только одно и разделение команд и запросов , потому что это заставляет объект изменять свое состояние, а также возвращать новое значение.

В общем, я считаю, что самым простым интерфейсом в этом случае будет:

public class Client {
  private float bal;
  float getBalance() { return bal; }
  void charge(float c) { bal -= c; }
}
MichelHenrich
источник
25
Пункт о разделении команд и запросов чрезвычайно важен, IMO. Представьте, что вы новый разработчик в этом проекте. Просматривая код, сразу видно, что getBalance()возвращает какое-то значение и ничего не меняет. С другой стороны, charge(0)похоже, что он что-то изменяет ... может быть, он отправляет событие PropertyChanged? И что это возвращает, предыдущее значение или новое значение? Теперь вы должны искать это в документации и тратить мозги, которых можно было бы избежать, используя более четкий метод.
Маг Xy
19
Кроме того, использование charge(0)в качестве способа получения баланса, потому что это, оказывается, не имеет никакого эффекта, теперь приводит вас к этой детали реализации; Я легко могу представить себе будущее требование, когда отслеживание количества сборов становится актуальным, и в этот момент ваша кодовая база изобилует платежами, которые на самом деле не являются сборами, становится проблемой. Вы должны предоставить элементы интерфейса, которые означают, что ваши клиенты должны делать, а не те, которые просто делают то, что нужно вашим клиентам.
Бен
12
Предупреждение CQS не обязательно уместно. Для charge()метода, если этот метод является атомарным в параллельной системе, может быть целесообразно вернуть новое или старое значение.
Дитрих Эпп
3
Я нашел ваши две цитаты для CQRS чрезвычайно неубедительными. Например, мне, как правило, нравятся идеи Майера, но просмотр возвращаемого типа функции, чтобы сказать, мутирует ли она что-либо, является произвольным и ненадежным. Многие параллельные или неизменные структуры данных требуют мутации + возврата. Как бы вы добавили строку, не нарушая CQRS? У вас есть лучшие цитаты?
user949300
6
Почти ни один код не соответствует разделению командных запросов. Он не идиоматичен ни в каких популярных объектно-ориентированных языках и нарушается встроенными API каждого языка, который я когда-либо использовал. Таким образом, описывать его как «лучшую практику» кажется странным; Можете ли вы назвать хотя бы один программный проект, который соответствует этому шаблону?
Марк Эмери
28

ИМО, заменив getBalance()с charge(0)через приложение не является упрощение. Да, в нем меньше строк, но он запутывает смысл charge()метода, который потенциально может вызвать головную боль, когда вам или кому-то еще понадобится вернуться к этому коду.

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

Мэтт Далзелл
источник
13

Важно помнить, что ваш код должен самодокументироваться. Когда я звоню charge(x), я ожидаю xоплаты. Информация о балансе является вторичной. Более того, я не знаю, как charge()это реализовано, когда я это называю, и я точно не буду знать, как это будет реализовано завтра. Например, рассмотрим это потенциальное будущее обновление для charge():

float charge(float c) {
    lockDownUserAccountUntilChargeClears();
    bal -= c;
    Unlock();
    return bal;
}

Внезапно использование charge()баланса не выглядит так хорошо.

Дэвид говорит восстановить Монику
источник
Да! Хороший момент, который chargeнамного тяжелее.
Дедупликатор
2

Использование charge(0);для получения баланса - плохая идея: однажды кто-то может добавить туда некоторый код, чтобы регистрировать произведенные платежи, не осознавая другое использование функции, и затем каждый раз, когда кто-то получает баланс, он будет регистрироваться как сбор. (Есть способы обойти это, например, условное утверждение, которое говорит что-то вроде:

if (c > 0) {
    // make and log charge
}
return bal;

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

Короче говоря: не полагайтесь ни на своих пользователей, ни на ваших последователей-программистов, понимающих, что charge(0);это правильный способ получить баланс, потому что если нет документации, которую они гарантированно не пропустят, то, откровенно говоря, это выглядит как самый пугающий способ получения баланса возможный.

Майкл Джонсон
источник
0

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

Shaz
источник
-2

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

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

(Это шаблон проектирования, для которого я не помню конкретного имени: определение сложного дружественного к абонентам интерфейса в терминах минимального дружественного к исполнителю. В Classic Mac OS минимальный интерфейс для операций рисования назывался «узким местом». но это не похоже на популярный термин.)

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

Кевин Рид
источник
1
Действительно, у меня были случаи, когда был выбран более минимальный интерфейс, чтобы облегчить полное тестирование покрытия. Но это не обязательно подвергается воздействию пользователя класса. Например, вставка , добавление и удаление могут быть простыми обертками общей замены, в которых все контрольные пути хорошо изучены и протестированы.
JDługosz
Я видел такой API. Распределитель Lua 5.1+ использует его. Вы предоставляете одну функцию, и на основе параметров, которые она передает, вы решаете, пытается ли она выделить новую память, освободить память или перераспределить память из старой памяти. Это болезненно , чтобы написать такие функции. Я бы предпочел, чтобы Lua только что дал вам две функции, одну для распределения и одну для освобождения.
Никол Болас,
@NicolBolas: И нет для перераспределения? Во всяком случае, у этого есть дополнительное «преимущество», которое почти такое же, как reallocиногда в некоторых системах.
Дедупликатор
@Deduplicator: распределение против перераспределения ... ОК. Это не очень хорошо, чтобы поместить их в одну и ту же функцию, но это не так плохо, как поместить освобождение в одну и ту же функцию. Это также легко сделать различие.
Николь Болас
Я бы сказал, что совмещение освобождения с (пере) распределением является особенно плохим примером интерфейса такого типа. (Deallocation имеет совершенно другой контракт: он не приводит к действительному указателю.)
Кевин Рейд