Здравствуйте, ·, Вы писали:
S>>Вам сходу придётся реализовывать концепт тайм-провайдера, и как-то привязывать его к вашей nextFriday().
·>Не знаю о чём ты тут говоришь. Выше по топику у меня был код тестов.
Простите, не нашёл. Вижу вот такой код:

class CalendarLogic {
   ...
   LocalDate nextFriday() {
      return doSomeComputations(instantSource.now());
   }
}

Вот тут у вас уже есть некий instantSource. Откуда он появился? Да оттуда, что интуитивно понятный код джуна не получилось протестировать:

class CalendarLogic {
   ...
   LocalDate nextFriday() {
      return doSomeComputations(java.time.LocalDateTime.now());
   }
}

Это был прекрасный код, который не устраивал только одним — невозможно проверить, что он работает корректно. Поэтому пришлось заменять статик метод зависимостью; потом мокать зависимость; потом — молиться, что мок адекватно отражает поведение LocalDateTime.

·>Ну перечитай топик что-ли.
Да я его читаю с самого начала. И коллега Pauel, хоть и ошибается в мелочах, в целом совершенно прав.

·>Разговор об этом не шел. Был именно "серверное время". Вот _если_ у меня появится требование с Request.Document.SignOffTime, вот тогда я и введу nextFriday(someDay), я же это уже написал ~час назад. Это вопрос автоматического рефакторинга "extract method" и нескольких минут от силы. Зато вся эта ботва с кешами и оптимизациями — будет видна как на ладони.
И я вам больше часа назад объяснил, в чём проблема этого рефакторинга. А ваши рассуждения продиктованы как раз тем, что вы не понимаете, что такое "серверное время" и "клиентское время".
Вопрос не в том, что есть различные "источники времени", которые производят новые инстанты императивным способом. А просто в том, что "время" — это величина, которая копируется из разных мест, но предсказуемым образом.
И если, скажем, у нас в коде стоит задача "сделать выписку за период от прошлого понедельника до следующей пятницы", то нам категорически противопоказано в коде этого отчёта вызывать lastMonday() и nextFriday(), реализованные в вашем стиле. Просто потому, что между этими вызовами код workflow может быть вытеснен на произвольное время — в том числе и такое, за которое результат nextFriday() изменится. \
Именно поэтому мы не делаем класс TransactionReport — наследник класса Report, оборудованный зависимостями от ScheduleCalculator, зависящего от InstantSource.
Мы делаем набор простых чистых функций:

(Start, End) GetReportDateRange(DateTime) // вычисляет начало и конец диапазона отчёта для заданной даты. Не задумывается, откуда берётся дата - зато внутри этой функции начало и конец диапазона всегда согласованы
DataQuery GetReportDataQuery(   // строит запрос для выборки данных по заданным параметрам. Пространство комбинаций велико, но счётно.
  UserID id,                    // Каждый из тестов выполняется мгновенно, так что мы можем себе позволить выполнять весь набор на каждый коммит
  Account[] accounts,           // а если у нас возникнет комбинаторный взрыв, то мы порежем эту функцию на куски
  DateTime start,               // превратив объём тестирования из произведения в сумму
  DateTime end) 
                                                                                          
                                                                                          
Transaction[] GetReportData(    // эту функцию вообще не надо тестировать, т.к. её реализовали другие люди, которые уже провели за нас подробнейшее тестирование. 
 DataQuery query                // Найти СУБД, которая даёт некорректные результаты по корректному SQL, практически невозможно (даже в MySQL осталось всего несколько известных глюков)
) 

XXX GetReportMarkup(Transaction[] ) // Это тоже чистая функция, которая прекрасно тестируется на различных примерах - пустой список, непустой список, етк.

byte[] RenderMarkupToPDF(ХХХ)   // опять чистая функция; протестировать её тяжело - но этим опять занимаются другие люди, которые могут себе это позволить
                                // потому что одна и та же функция используется в тыщах мест - и мы переносим усилия по тестированию каждого отдельного отчёта в тесты рендерилки PDF.

Всё. У нас юнит-тестами покрыто 100% нашего кода; а код вне зоны нашей ответственности даже не исполняется во время наших тестов.
Теперь остаётся всё это склеить. Риск при склейке минимален — просто потому, что в ней очень мало кода:

DateTime.now()
  .GetReportDateRange()
  .GetReportDataQuery()
  .GetReportData()
  .GetReportMarkup()
  .RenderMarkupToPDF()
  .SendTo(request.GetResponseStream();

Я не отказываю этому коду в шансе содержать ошибку — действительно, мы могли напороть примерно везде. Например, неверно поняли требования, и в маркапе есть косяк (скажем, числа в таблице выровнены влево, а не по десятичной точке). Или, скажем, мы использовали как раз не то время — между подачей запроса на отчёт и началом работы обработчика как раз минула полночь на первое, и вместо отчёта за январь 2024 пользователь получает отчёт за февраль, в котором 0 строк (ведь февраль еле успел начаться).
Такие вещи ловятся интеграционным тестированием, которое один хрен нужно проводить. Но к моменту, когда у нас дошла очередь интеграционного тестирования, где (медленно!) тащатся реальные данные из реальной базы, выполняется (медленный!) UI workflow с навигацией от логина до этого отчёта, и т.п., у нас уже есть очень-очень хорошая уверенность в том, что как минимум логика вычисления дат, логика построения запроса, и логика порождения разметки верна. Нам не нужно ждать развёртывания боевой системы, или даже заполнения in-memory database для того, чтобы поймать какой-нибудь "<" вместо ">" или ошибку склеивания двух предикатов в SQL.
На то это и unit тесты, что они тестируют поведение отдельных юнитов, у которых у самих нет состояния, и на состояние других они тоже не полагаются никак.

S>>Но на практике это превращается в we wish you a happy debugging, потому что конфиги теперь становятся частью решения, а для них нет ни средств рефакторинга, ни измерения тестового покрытия.
·>Какие конфиги?
Не знаю. А откуда у вас в CalendarLogic берётся instantSource?

·>Зависимости собираются в Composition Root. Тоже уже обсуждалось.
Ага, то есть у нас где-то есть тот самый код, который выполняет вот это вот

var cl = new CalendarLogic(new LocalDateTimeSource());
var nf = cl.nextFriday();

И вы почему-то думаете, что это как-то сильно понятнее, очевиднее, и лучше, чем

var nf = CalendarUtils.nextFriday(LocalDateTime.now());

·>Ты почему-то решил, что добавить параметр в NextFriday является какой-то проблемой. Совсем наоборот — добавить просто, т.к. это новое требование и новый код. Убрать или зафиксировать — сложнее, т.к. надо исследовать все варианты и места использования во всём существующем коде.
Ну так задачи убрать или зафиксировать — нету. И простота с добавлением — кажущаяся.

·>Да даже с таким кодом — я не понял где тут усложнение тестирования. В худшем случае пара лишних new, которые наверняка вырежутся оптимизатором.
Вы для начала покажите, как вы протестируете код, апдейтящий ваш кэш.

·>Если ты имеешь в виду как выражать свойства самого timeSource — в терминах ООП будет другой тип MonotonicTimeSource. И конструктор|фабрика NextFridayProvider может для разых подтипов TimeSource выбирать разные подходы к кешированию.
Да не будет у вас никакого другого типа. Вы уже навелосипедили код так, как навелосипедили — потому что не знали о требованиях монотонности.
Вот когда вы видите функцию nextFriday(DateTime) — вы почему-то сразу задумываетесь о том, как она будет работать, если в неё подаются произвольные значения аргумента.
А пока вы мыслили в категориях "как бы мне замокать now()", вы навелосипедили вот это:

class CalendarLogic {
   private volatile LocalDate currentFriday;//вот наш кеш
   constructor... {
      var updater = () => currentFriday = doSomeComputations(instantSource.now());
      timer.schedule(updater, ... weekly);
      updater.run();
   }
   ...
   LocalDate nextFriday() {
      return currentFriday;
   }
}

И это у вас прекрасно грохнется в продакшне, как только на машине сменят таймзону аккурат около полуночи пятницы. Обратите внимание — в моём коде вы усмотрели проблему перформанса; ваш код некорректен функционально.
Понимаете? Автор исходной CalendarLogic считает, что всё ок — он же всего лишь заменил now() на instantSource для удобства тестирования.
Автор тестов CalendarLogic считает, что всё ок — ну как же, у него code coverage == 100%, все бранчи протестированы, сломаться нечему.
Автор изменения CalendarLogic считает, что всё ок — он же прогнал все тесты; регрессии нету. Он даже (наверное) как-то протестировал, что updater реально исполняется так, как он ожидает.

·>Что значит случайно? Это тестами покрывается.
Ну, так вы напишете такой тест, в котором instantSource возвращает немонотонные значения? А такой, который ломает ваш CalendarLogic, процитированный выше?


S>>Тут вы попадаете в ловушку. Как раз у Pauel никакого изменения дизайна не случится, потому что у него nextFriday с самого начала имела нужную ему сигнатуру.
S>>А вы теперь будете вынуждены продублировать код nextFriday — ну, либо всё же перенести его в новую nextFriday, а старую переделать на вызов новой.
·>Аж два рефакторинга "extract method", "move method". Всё. 2 минуты времени.
Отож. И плюс новый набор тестов. При этом старые тесты тоже нужно гонять, удваивая время исполнения. Прикольно, чо. Поднимаем себестоимость на ровном месте.

·>Тесты как проходили, так и будут проходить. Впрочем, наверное после этого стоит тесты перенести тоже, для красоты.
Что значит "перенести"? А новый код старой функции у вас что, будет непокрытым?
А как тогда вы защититесь от будущего джуна, который там что-нибудь отвинтит?

·>Да. Таймеры мочить надо в любом случае. Иначе как _вы_ это будете тестировать? Деплоить всё на сервера и ждать пятницы? Или системные часы в операционке перематывать?
Я это никак не буду тестировать. Потому, что мне даже не придёт в голову такая хрупкая архитектура. Делать обновление кэша по таймеру — одна из худших идей, которые только можно придумать.
·>Я, конечно, имею в виду, что мы оба сравниваем одинаковые подходы с обновлением кеша по таймеру.
Не, я не хочу сравнивать одинаковые подходы. Вы просто наглядно показываете, что ваш способ мышления примерно такой:
1. Сначала вы выбираете неудачный подход
2. Когда вам говорят про его недостатки, вы заявляете, что недостатков там нет
3. Когда вам недостатки указывают, вы придумываете решения, которые делают код хуже
4. Повторяем с п.2.

Твой подход с обновлением кеша в момент первого вызова я тоже могу реализовать, без использования таймеров естественно. Но это не универсальный всемогутер, операционные недостатки такого подхода я уже упомянул.
С моей точки зрения, там операционные недостатки в 10-1000 раз меньше, чем один внедрённый в задачу таймер. Во-первых, там код работает корректно, в отличие от кода с таймером.
Во-вторых, он легко обобщается на случай "незначительной" немонотонности (например, иногда подают дату с прошлой недели, хотя в основном — с этой).
В-третьих, там этот "спайк задержки" работает на 1 запрос. Практически невозможно придумать SLA, на которое это повлияет.
Например, если у нас требования "99% запросов укладываются в X", при этом поток запросов велик — спайк попадёт в 1% и на процентиль не повлияет.
Если у нас поток запросов низкий, то, собственно, выигрыш за счёт кэширования вряд ли будет значительным. Нам полезнее будет покрутить реализацию "SlowNextFriday", чтобы она стала менее slow.
Ну, и в самом крайнем случае, мы тоже можем привинтить к FP-коду таймер, если это окажется единственным выходом.

·>Я же показал где хрупкость связи. До строчки nextFriday = wrapAndCacheMonotonous(nextFriday); писать nextFriday(platezhka.date) можно, а после только nextFriday(Time.Now) и больше никак.
Почему никак? Вполне можно. Код продолжит корректно работать. Просто иногда он будет чуть медленнее исполняться. Если это создаст проблему — то мы это обнаружим.
А, главное, все эти связи — ровно в одном месте, у нас перед глазами. Нет никаких неявных зависимостей и сложных соотношений. platezhka.date написано примерно там же, где и wrapAndCacheMonotonous. Поэтому шансы на то, что я там напорю, минимальны. Но главное не в этом — а том, что в вашем подходе нет никаких преимуществ. Ну нету у вас способа гарантировать монотонность источника времени. Недостаточно выразительная система типов, увы.
И рассказы о том, что вы будете делать рефакторинг вашего CalendarLogic, я воспринимаю скептически. Вы — может быть и будете, вы автор этого кода, вам с ним комфортно.
А кто-то другой, например джун, которому поручили починить проблему, пойдёт по пути наименьшего сопротивления: ага, видим, что гуру . уже спроектировал CalendarLogic так, чтобы он умел брать дату из внешнего источника.
Ну прекрасно — мы просто скормим ему вместо SystemLocalDateSource экземпляр нового класса PlatezhkaSource(platezhka), у которого now() перегружен соответствующим образом.
Это ж краеугольный камень ООП — все проблемы решаем при помощи агрегации и полиморфизма.

·> Открой для себя паттерн Декоратор.
Да я в курсе про паттерн Декоратор. Я, если чо, несколько лет преподавал объектно-ориентированный анализ и дизайн в университете
Но вот в вашем коде, обратите внимание, нет никакого декоратора. Это опять означает, что вас надо бить лопатой для того, чтобы вы сделали код, хотя бы приблизительно дотягивающий до кода, который ФП-программист пишет инстинктивно, т.к. в ФП такая идиоматика.
·>Насчёт stateless я не спорю, тут полностью согласен. Но не всегда есть такая роскошь.
Тут главное — знать, к чему стремиться. Пока что выглядит так, что вы на ровном месте пишете stateful не потому, что иное невозможно, а так — на всякий случай. Аргументируя в стиле "зато когда потом, в будущем, мне понадобится стейт — вот он, у меня уже есть!". А заодно вы любите протаскивать зависимости от стейтфул-объектов.
А надо — наоборот: зависимостей — избегать, стейта — избегать.
Понадобится стейт — всегда можно добавить его по необходимости.

S>>·>Я не про монотонность, а про то, что в nextFriday(x) — есть степень свободы что передавать в качестве x — и приходится делать предположения и анализ что может быть этим x во всём коде. В моём случае у меня явное ЧПФ nextFriday = () => nextFriday(timeSource.now()), зафиксированно знание о том, что же может быть этим самым x есть более широкий простор для оптимизаций.
Нет у вас никакого широкого простора для оптимизаций. Есть иллюзии того, что timeSource.now() ведёт себя как-то иначе, чем аргумент (x). И, естественно, оптимизации, построенные на этих иллюзиях, будут приводить к фейлам.

·>Это называется code reuse. Если у меня в 100 местах нужно посчитать nextFriday для платёжки, то я заведу именно такой метод nextFriday(platezhka), а не буду в 100 местах копипастить nextFriday(platezhka.date). Потому что в моей системе в первую очередь важны бизнес-документы, а не алгоритмы 14го века.
Ну так это если. А вы всё время пытаетесь решить проблему, которая ещё не возникла, как правило, привнося проблему, которой не было.

·>Нет. Метод, который процессит ордер будет выглядеть примерно так processMessage(ByteBuffer order) и зваться напрямую из сетевого стека. Откуда в сетевом стеке возьмётся вся эта инфа о биржах — я не знаю. Сетевой стек тупой — пришёл пакет из сокета X, дёргаем метод у привязанного к этому сокету messageHandler.
Это вы играете словами. Всё равно внутри processMessage(ByteBuffer buffer) будет декодирование этого buffer, и диспетчеризация по его содержимому.
То, что вы хотите это делать при помощи хрупкого набора ссылающихся друг на друга стейтфул-сущностей — ваше право. Я даже могу себе представить ситуацию, где это будет оправдано.
Но в большинстве случаев даже для такой задачи т.н. anemic model окажется сильно более пригодной, чем любая иерархия толстых сущностей в описываемом вами стиле.

S>>Что, в частности, позволяет нам лёгким движением руки выполнить replay пятничной сессии в понедельник, если в коде биржи обнаружилась ошибка и мы вынуждены аннулировать результаты торгов.
·>Для этого просто подменяется timeSource. Во время replay он будет не system clock, а timestamp из event log.
Ну прекрасно — ваш CalendarLogic, завязанный на таймер, опять выдал неверный результат. Несмотря на успешные юнит-тесты
Даштоштакое-то!

·>Естественно, т.к. это типичный глобальный синглтон. Поэтому и изобрели InstantSource. И если я правильно понимаю Pauel (код он как всегда скрывает), он предлагает писать DateTime.Now() в каждом методе контроллера, т.к. именно там у него интеграция всего со всем.
Совершенно верно — это самый нормальный способ писать контроллер.
Вместо того, чтобы костылить себе путь при помощи повышения уровня косвенности.