«Запах» проектирования: одержимость примитивами

[VolCh]

Как-то надумано, имхо. Вместо сеттера или конструктора, проверяющего значение на допустимость, вводить новый класс, единственной ответственностью которого будет хранение допустимого значения? А при изменении условия в первом классе лезть во второй, плюс убедиться что на другие классы изменения во втором не повлияют? Или вводить дополнительное условие в первом? Или создать третий (может независимый от второго, может наследник)?

[qw1]

Ну хотя бы не получится из-за опечатки присвоить IP-адресу номер телефона, которые обычно складывают в одинаковый string.

[VolCh]

Речь, по-моему, не о том, проверять ли данные на валидность, а том где эту проверку проводить — в сеттере или конструкторе основного класса или в конструкторе или сеттере класса, специально под это выделенного.

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

[qw1]

Compile-time проверка априори лучше run-time.
Кто знает, когда выполнится эта ветка кода с опечаткой.

[VolCh]

На моих основных языках compile-time по сути отсутствует, но разве компилятор С* будет проверять что я передаю в коде конструктору или сеттеру класса типа Money? Строку «123 USD» или строку "[phone]"?

[qw1]

как минимум, это защитит от случайной перестановки параметров при вызове функции в коде

Call CreateCall(PhoneNumber dstNum, CountryCode from) { ... }
...
c = CreateCall(user.num, user.country);

[qw1]

На моих основных языках compile-time по сути отсутствует

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

[StreetStrider]

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

Именно! Именно единственной ответственностью.

[VolCh]

Но она будет искусственно отделена от ответственности основного объекта хранить себя в непротиворечивом состоянии. И это выделение не исключает дополнительных проверок. Скажем есть кредитный счёт, у суммы которого два ограничения: неотрицательная и не больше кредитного лимита клиента. Введя тип Money вместо примитивного, мы можем контролировать в нём только неотрицательность, а проверку на лимит всё равно придется проводить в основном объекте. Есть ли смысл разделять проверку amount >= 0 && amount <= this.limit на две в разных объектах?

[StreetStrider]

Понял вас. В любом случае сущности Money и Account в данном примере будут связаны, вопрос в том, насколько явной мы сможем сделать эту связь. Ведь можно проверять и в Account, но нам потребуется делать только нетривиальные проверки для данного поля, делегировав вещи типа amount >= 0 в примитив. Такие типы ведь включают в себя ещё и значения по умолчанию, сообщения об ошибках. Дальше не придётся тащить это всё за собой в каждый класс, а можно будет просто повторно использовать. В случае если вплавлять эти проверки в класс, то мы сможем реюзать это только наследуюясь.

[VolCh]

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

[mayorovp]

Некорректный пример. Кредитный счет может иметь отрицательную сумму (после какого-нибудь перерасчета или возврата средств) или сумму, превышающую лимит (после установки нового лимита или у тех, кто свои кредиты просрочил). В итоге, проверки amount >= 0 и amount <= this.limit редко когда должны проверяться совместно.
Так, когда мы погашаем кредит, глупо проверять лимит — в итоге может получиться ситуация «вы не можете погасить кредит, так как после операции погашения ваш кредитный лимит будет превышен» — такая ситуация может возникнуть, если и до погашения лимит был превышен. Аналогично, и с проверкой на неотрицательность — если по какой-то причине банк был должен клиенту, то глупо запрещать брать тому кредит из-за того, что сумма кредита слишком маленькая.

И да, деньги могут ходить в обе стороны, а потому включать любую из этих проверок в класс Money было бы странно. Но в класс Money можно включить проверку того, что передаваемая сумма кратна копейке — такая проверка будет полезной.

[VolCh]

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

if ($amount < 0) {
  throw new NegativeAmountException;
} elseif ($amount > $this->limit) {
  throw new LimitOutAmountException;
} 

. Счет с превышенным лимитом недопустим также — если появляется эксепшен об этом, то это повод для совместного расследования СБ и ИТ.

[mayorovp]

Счет с превышенным лимитом недопустим также — если появляется эксепшен об этом, то это повод для совместного расследования СБ и ИТ.

А каким будет состояние счета в процессе проведения проверки? Сможет ли клиент такой кредит погасить до ее окончания? Будут ли ему начислены проценты за то время, когда он не мог погасить кредит из-за длящейся проверки?

[VolCh]

Не изменится. Факт внесения денег в кассу будет зафиксирован, но счёт автоматически не изменится. Если проверка покажет, что произошла ошибка, то кредит будет погашен задним числом.

[mayorovp]

Хм, странная модель, но, наверное, она работает.

[VolCh]

У нас вообще много странного :)

[Anthony]

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

[EngineerSpock]

Недавно прочиталал в «Growing object oriented software guided by tests» про случай, когда NASA, разрабатывавшая программу для какого-то комического изделия, который должен был лететь на Марс, пострадали как раз из-за того, что хранили различные метрики, отличающиеся по смыслу в примитивах (числовых). Кто-то где-то спутал концепции и в результате произошла «катастрофа». Авторы рекомендует выделять концепции в отдельные классы в любом случае, даже если там проверок никаких не будет. Имя класса или структуры делает акцент на том, что есть некая сущность, представляющая концепт.

[iroln]

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

[qw1]

В C++ нет никакого оверхеда, если я опишу классы

class phone_number : public string { }

class FileHandle { public: uint32_t handle; }

Причём, во втором случае, при передаче по значению, компилятор обращается с типом, как с обычным int: например, передаёт в функции значение handle в регистре CPU.

[EngineerSpock]

Катастрофа может произойти по любой причине, но, как утверждают авторы, в данном случае причина была именно в этом. А мы стараемся же избегать катастроф, да? А про излишние накладные расходы, исключая того, что вам уже сказали относительно С++, какие накладные расходы больше — лишний вызов или пара миллиардов долларов в случае катастрофы? (с учётом пункта — делайте абстракции для концепций, а не используйте примитивы)

p.s. Промазал уровнем, извиняюсь, утро — не проснулся ещё)))

[micbsv]

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

[micbsv]

Кстати, огромное спасибо за статьи! Программирую более 15 лет и очень интересно осваивать чужой опыт.