Здравствуйте, Sinclair, Вы писали:

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

Автор: ·
Дата: 28.12.23

.

S>Это был прекрасный код, который не устраивал только одним — невозможно проверить, что он работает корректно.
Это то что рассказывает Pauel у него происходит:

·>Так как именно ты собираешься в "curl" и-тестах ассертить, что туда передаётся именно текущее время?
Никак.

S>Поэтому пришлось заменять статик метод зависимостью; потом мокать зависимость; потом — молиться, что мок адекватно отражает поведение LocalDateTime.
Ну это то что Pauel предлагает, засовывая Time.now в контроллеры, ставит мне в пику, что я это мокаю: " Во вторых, чуть далее, тоже недавно, вы рассказывали, как моками отрезали Time.now".

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

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

S>Просто потому, что между этими вызовами код workflow может быть вытеснен на произвольное время — в том числе и такое, за которое результат nextFriday() изменится. \
Ну ясен пень. Это слишком простой вопрос, не стоящий обсуждения. Ну да, мы должны получить некий момент и от него плясать по бизнес-логике. Скажем, тот же TradeDate нужно взять только один раз для данного ордера, в момент его прихода в handler, запомнить его и плясать от него далее, вычисляя всякие там settlement date, days to maturity и т.п.

А вопрос об источниках времени — это интересный вопрос, именно его я и обсуждаю. И что один и тот же код, как в недавнем примере, может работать с системным источником RTC в боевой системе, а потом с источником времени из event log во время replay, а ещё и с тестовым источником времени (мок!) во время прогона тестов, чтобы можно было тестировать сценарии "подождём до конца финансового года".
Вот это интересное обсуждение.

Я в первую очередь говорю о главном применении моков — отрезать входы-выходы системы (тот самый Time.Now, а ещё сеть, фс, бд, етс), которые в реальности завязываются на что-то тяжело контролируемое или очень медленное.
Вы же с Pauel рассуждаете о скучных нутрах бизнес-логики. Там всё тривиально.

S>Такие вещи ловятся интеграционным тестированием, которое один хрен нужно проводить. Но к моменту, когда у нас дошла очередь интеграционного тестирования, где (медленно!) тащатся реальные данные из реальной базы, выполняется (медленный!) UI workflow с навигацией от логина до этого отчёта, и т.п., у нас уже есть очень-очень хорошая уверенность в том, что как минимум логика вычисления дат, логика построения запроса, и логика порождения разметки верна. Нам не нужно ждать развёртывания боевой системы, или даже заполнения in-memory database для того, чтобы поймать какой-нибудь "<" вместо ">" или ошибку склеивания двух предикатов в SQL.
Вот это я не понял. У него в примере тест буквально сравнивал текст sql "select * ...". Как такое поймает это?
Суть в том, что программист пишет код+тест. В тесте вручную прописывает ожидание =="sellect * ...". Потом тест, ясен пень проходит, потом ревью, который тоже наверняка пройдёт, не каждый ревьювер способен заметить лишнюю букву. И только потом, когда некий тест пройдёт от логина до отчёта, то оно грохнется, может быть, если этот тест таки использует именно эту ветку кода для тестовых параметров отчёта. Ведь итесты в принципе не могут покрыть все комбинации, их может быть просто экспоненциально дохрена.

S>На то это и unit тесты, что они тестируют поведение отдельных юнитов, у которых у самих нет состояния, и на состояние других они тоже не полагаются никак.
С этим осторожнее. Вот этот твой кеш оказался — с состоянием, внезапно, да ещё и static global.

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

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

SINGLE line composition root:

S>var cl = new CalendarLogic(new LocalDateTimeSource());

logic in many-many places, hundreds lines:

S>var nf = cl.nextFriday();

CompositionRoot — один на всё приложение, меняется относительно редко, обычно тупой линейный код, и составляет доли процента от объёма всего приложения. Его можно ежедневно глазками каждую строчку просматривать всей командой.

Даже больше. Composition Root можно делить на модули. Будет модуль типа:

class ApplicationModule
{
   public ApplicationModule(TimeSource timeSource)
   {
     var cl = new CalendarLogic(timeSource);
     var reportLogic = new ReportLogic(timeSource);
     var blahThing = new BlahThing(cl, reportLogic, 42);
     var businessLogic = new BusinessLogic(blahThing, timeSource);
     var restHandler = new RestHandler(businessLogic, "qwt");
   }
}

И вот такой модуль можно тестировать простыми тестами, подсовывая туда моковый timeSource и ровно этот же модуль будет зваться из "main" метода реального приложения, куда будет запихан SystemClock в качестве timeSource.

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

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

S>
И это по всему коду — везде в тысячах мест. Да? И как это тестировать? Как делать replay?

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

S>·>Да даже с таким кодом — я не понял где тут усложнение тестирования. В худшем случае пара лишних new, которые наверняка вырежутся оптимизатором.
S>Вы для начала покажите, как вы протестируете код, апдейтящий ваш кэш.
Я обычно использую специальный тестовый шедулер, который запускает установленные таймеры при изменении тестового источника времени. У есть метод типа tickClock(7, DAYS). Код писать лень, но идея, надеюсь, очевидна...

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

S>И это у вас прекрасно грохнется в продакшне, как только на машине сменят таймзону аккурат около полуночи пятницы.
Нет, конечно. Таймеры ставят на instant, т.е. абсолютный момент времени, независящий от зоны.

S>Обратите внимание — в моём коде вы усмотрели проблему перформанса; ваш код некорректен функционально.
Ну я код в браузере набирал, мог и ошибиться. В реальности буду запускать тесты, ясен пень.

S>·>Что значит случайно? Это тестами покрывается.
S>Ну, так вы напишете такой тест, в котором instantSource возвращает немонотонные значения?
Через моки же — возвращай что хошь в каком хошь порядке.

S>А такой, который ломает ваш CalendarLogic, процитированный выше?
В сымсле ботва с таймзонами? Ну это у меня уже привычка — писать тесты для дат около всяких dst переходов и с экзотическими таймзонами.

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

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

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

S>·>Да. Таймеры мочить надо в любом случае. Иначе как _вы_ это будете тестировать? Деплоить всё на сервера и ждать пятницы? Или системные часы в операционке перематывать?
S>Я это никак не буду тестировать. Потому, что мне даже не придёт в голову такая хрупкая архитектура. Делать обновление кэша по таймеру — одна из худших идей, которые только можно придумать.
Ну не нравится, не ешь. Об чём спор? Для чего нужен таймер — я тебе написал, для low latency. Без таймера твой вариант у меня тоже элементарно реализуется, правда мне совесть не позволит убить ещё одного котёнка делая static глобальную переменную. Можно даже гибридное решение наворотить, обновляя кеш по таймеру из другого треда, но если вдруг момент timeSource.now() промазывает по кешу, то идти по медленному пути.

S>·>Я, конечно, имею в виду, что мы оба сравниваем одинаковые подходы с обновлением кеша по таймеру.
S>Не, я не хочу сравнивать одинаковые подходы. Вы просто наглядно показываете, что ваш способ мышления примерно такой:
S>1. Сначала вы выбираете неудачный подход
Он удачный в определённых условиях. То что твой подход "удачный" и универсальный всемогутер — твоё большое заблуждние.

S>2. Когда вам говорят про его недостатки, вы заявляете, что недостатков там нет
S>3. Когда вам недостатки указывают, вы придумываете решения, которые делают код хуже
S>4. Повторяем с п.2.


S>·>Твой подход с обновлением кеша в момент первого вызова я тоже могу реализовать, без использования таймеров естественно. Но это не универсальный всемогутер, операционные недостатки такого подхода я уже упомянул.
S>С моей точки зрения, там операционные недостатки в 10-1000 раз меньше, чем один внедрённый в задачу таймер. Во-первых, там код работает корректно, в отличие от кода с таймером.
Это как? Если у нас в SLA написано, что max response time 10ms, а SlowlyCalculatePrevAndNextFridays работает 11ms, то выбора тупо нет, и похрен всем твоя точка зрения.

S>В-третьих, там этот "спайк задержки" работает на 1 запрос. Практически невозможно придумать SLA, на которое это повлияет.
Открой для себя мир low latency.

S>·>Я же показал где хрупкость связи. До строчки nextFriday = wrapAndCacheMonotonous(nextFriday); писать nextFriday(platezhka.date) можно, а после только nextFriday(Time.Now) и больше никак.
S>Почему никак? Вполне можно. Код продолжит корректно работать. Просто иногда он будет чуть медленнее исполняться. Если это создаст проблему — то мы это обнаружим.
У нас это было серьёзный production incident. Каждый запрос, обработка которого была >10ms — тикет и расследование. Больше таких двух в день — и клиент нам звонит для разборок.

S>А, главное, все эти связи — ровно в одном месте, у нас перед глазами. Нет никаких неявных зависимостей и сложных соотношений. platezhka.date написано примерно там же, где и wrapAndCacheMonotonous. Поэтому шансы на то, что я там напорю, минимальны. Но главное не в этом — а том, что в вашем подходе нет никаких преимуществ. Ну нету у вас способа гарантировать монотонность источника времени. Недостаточно выразительная система типов, увы.
S>И рассказы о том, что вы будете делать рефакторинг вашего CalendarLogic, я воспринимаю скептически. Вы — может быть и будете, вы автор этого кода, вам с ним комфортно.
S>А кто-то другой, например джун, которому поручили починить проблему, пойдёт по пути наименьшего сопротивления: ага, видим, что гуру . уже спроектировал CalendarLogic так, чтобы он умел брать дату из внешнего источника.
S>Ну прекрасно — мы просто скормим ему вместо SystemLocalDateSource экземпляр нового класса PlatezhkaSource(platezhka), у которого now() перегружен соответствующим образом.
S>Это ж краеугольный камень ООП — все проблемы решаем при помощи агрегации и полиморфизма.
Это повлияет только на новый код с платёжкой. Подменить "случайно" по всему приложению так не выйдет. А вот твоя глобальная переменная — happy debugging.

S>·> Открой для себя паттерн Декоратор.
S>Да я в курсе про паттерн Декоратор. Я, если чо, несколько лет преподавал объектно-ориентированный анализ и дизайн в университете
S>Но вот в вашем коде, обратите внимание, нет никакого декоратора. Это опять означает, что вас надо бить лопатой для того, чтобы вы сделали код, хотя бы приблизительно дотягивающий до кода,
У меня была цель продемонстрировать не основы объектно-ориентированного анализа, а факт, что можно максимально оптимизировать реализацию nextFriday(), т.к. у нас известно многое внутри самого метода. Навешивая это всё снаружи, приходится делать всё более пессимистично, т.к. инфы тупо меньше.

S>который ФП-программист пишет инстинктивно, т.к. в ФП такая идиоматика.
Ага, смешно у Pauel инстинкт с memoize сработал.

S>·>Насчёт stateless я не спорю, тут полностью согласен. Но не всегда есть такая роскошь.
S>Тут главное — знать, к чему стремиться. Пока что выглядит так, что вы на ровном месте пишете stateful не потому, что иное невозможно, а так — на всякий случай. Аргументируя в стиле "зато когда потом, в будущем, мне понадобится стейт — вот он, у меня уже есть!". А заодно вы любите протаскивать зависимости от стейтфул-объектов.
S>А надо — наоборот: зависимостей — избегать, стейта — избегать.
Ага, и ты тут такой: static (DateTime prevFriday, DateTime nextFriday)? _cache А потом хрясь и в догонку nextFriday(LocalDateTime.now())!

S>Понадобится стейт — всегда можно добавить его по необходимости.
А вот попробуй избавиться от этого static и у тебя полезет колбасня.

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

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

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

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

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