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

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

M>>Как уже правильно сказали, в readme не написано, как собирать. Одна-две строчки много времени не займут, а для читающего приятно и не надо вспоминать, какая там у вас культура.
A>камон. я не для домохозяек туториал пишу.
Не для домохозяек. Но вы предполагаете, что проверяющий хорошо знает все модные тренды во всех трех предложенных языках. А это может быть и не так. Может, они Java позволяют только для того, чтобы иметь больший круг кандидатов и потом переучивать их на используемый стек. Можно не иметь подробного описания сборки для проектов внутри команды, общее знание достигается через onboarding и тренировку. А вот для тестового задания две-три строчки о сборке будут полезны. Еще показывают, что вы все-таки задумываетесь о пользователях и умеете в какой-то мере бороться с проекцией "я знаю, значит и все остальные должны знать".

M>>У вас там в бинарниках трояна нет?
A>это тестовое задание. как и что это показывает — от "ничего" вплоть до "все"
Но вы же согласились его делать. Значит согласны, что что-то оно показывает? Свобода выбора технологий как раз полезна — показывает, о каких глубоких вопросах задумывался кандидат. И какие предпочтения имеет.

M>>Тесты. Вы их с падающими assert проверяли? У меня вот подозрение, что после первого падения все остальное упадет с AddressAlreadyInUse. Что как-то нехорошо. Например потому, что после этого нужно исправлять первый упавший тест а не любой понравившийся.
A>проверял
Ну и как? Удобно находить, где там ошибка? Я вот специально сейчас сломал один тест. Получил четыре AssertionError без каких-либо внятных сообщений об ошибках. Пришлось копаться в стеках, чтобы понять, откуда они происходят. Дополнительные сложности там, где их легко избежать.

M>>Не StorageTest а StorageImplTest. Вот представьте, у вас есть несколько реализаций Storage. Какую из них тестирует StorageTest?
A>вы серьезно?
Конечно. StorageTest по названию — это тест на соответствие спецификации (compliance). Т.е. любая реализация должна проходить эти тесты (которые с большой вероятностью property based). Здесь же можно поговорить про то, как сложно делать тесты, параметризованные объектами. При этом тесты конкретных реализаций либо расширяют StorageTest (и обходят ограничения фреймворков тестирования), или тестируют какие-то дополнительные свойства, характерные для этих реализаций.

M>>Ваш restful тянет только на Level 0 (используется HTTP). Банальный RPC over HTTP. SOAP, кстати, тоже попадает в REST Level 0. Хотелось бы видеть хотя бы Level 1 — Entities & Verbs (т.е. PUT /accounts/abc, GET /accounts, ...).
A>кому хотелось и зачем?
Ну в первую очередь вам. Потому что вы хотели продолжения интервью или найма. Хотелось бы мне. Как минимум потому, что приятно поговорить с кандидатом с широким кругозором. За реализацию Rest Level 1 вы бы получили плюсик. А на самом деле при создании API вы скорее всего вспомнили бы причины, по которым REST был спроектирован в его виде. И задумались бы про идемпотентность (idempotency) операций. Не обязательно ее реализовывать, можно было просто упомянуть в readme: "идемпотентность не реализована из-за ограничений на время, могу прислать план реализации по запросу". И получили бы сразу пять плюсиков.

Idempotency здесь очень важна. Вот представьте, я вызвал ваш RPC API и получил IOException. Вот что я с ним должен делать? Ничего (и рисковать, что запрос был потерян)? Или повторять запрос (и рисковать, что я проведу операцию дважды)? Вот из-за вашего API мне придется городить Artificiall Intelligence (искуственную разведку), которая будет выяснять состояние предыдущей операции прежде чем повторять. А с идемпотентным REST я бы ее просто повторил и сказал вам большое спасибо. Ситуация вполне реальная, и в том числе в финансовой сфере . Это — основная причина, по которой Rest Level 1 хотелось бы видеть лично мне.

M>>Ожидание ввода в main — решение спорное. Как оно поведет себя в каком нибудь Kubernetes? Там некому будет вводить порт. Вообще право на жизнь оно имеет, но такие сюрпризы должны описываться в readme.
A>надеюсь вы несерьезно, потому что тест не для запуска в Kubernetes
Серьезно. Потому что кандидат, написавший 5 приложений под Kubernetes, не будет вообще писать ввод. И опишет аргументы командной строки в README. Может это и не страшно, зависит от конкретной позиции, на которую вы претендуете.

M>>*Impl. Это вот у вас ровно одна реализация предполагается? Почему бы не InMemoryStorage, например?
A>а у вас? и почему если не одна, то не InMemoryCuncurrentHashSetStorageImpl? понятно куда клоню?
А у меня — несколько реализаций предполагается. И обычно их и есть несколько. Очень часто отдельная in-memory (или какая-то упрощенная) для тестов. Во многих случаях — декораторы. За что-то более осмысленное чем *Impl получили бы плюсик. Против вашего длинного варианта ничего не имею, он выше минимальных ожидаемых стандартов. За последовательное следование вашей конвенции (и сохранение нужных уровней абстракции и т.п.) получили бы сразу два плюсика. И я бы начал задумываться, а не стоит ли внедрить ваш стиль для всей команды.

M>>Я бы вообще эти реализации спрятал за Factory Method. Потому что тогда их можно по желанию менять, разбивать/переразбивать на разные классы и т.п. В конце концов, все использования идут только через реализованные интерфейсы.
A>нечего добавить. не вижу в 2019 году задачи явно показывать знание Factory Method
А вот и не угадали . Это задача на проектирование API, минимизирующего изменения в кодовой базе и расширяющего круг возможных рефакторингов. Один конкретный случай, конечно, не влияет. Но ведь проектирование таких API — это автоматизированный навык (подобных решений разработчики делают много). А вот на большой кодовой базе преимущества factory method уже будут заметны. Опять же, я на самом деле не требую именно фабричный метод (это один плюсик). Любое другое API, служащее той же цели (минимизация потенциальных изменений и расширение простора для рефакторинга) даст вам как минимум два плюсика.

M>>В StorageImpl вложенность кода глубокая. Можно было бы спрямить через if/throw и if/return.
A>а можно было и нет. дело вкуса
Это конечно да. Но толерантность к глубоким стрелкам настроаживает. Очень часто приводит к длинным и глубоким вложенностям, условиям и т.п. А потом мне приходится вспоминать, к чему же относится одинокий else с одной строчкой после большого блока кода, где его if не влез на его страницу. Чем меньше надо помнить — тем лучше.

M>>Account интересен. Почему у вас конструктор по умолчанию создает счет с невалидным id?
A>а с каким валидным id он может его создавать? это издержки json сериализации, нужен дефолтный конструктор, и вопросвалидности отдельный и серьезный
Вот! Именно таких вопросов (можно для начала и без ответов) я и жду от вас! Можно описать вопросы валидности в kotlindoc. Можно — в readme проекта. Можно взять сериализатор, не страдающий издержками (это тестовое задание, я хочу видеть, как вы видите идеальный код!). Пусть даже не распространенный, не важно. Я очень хочу иметь в команде разработчиков, видящих много факторов и находящих компромисс между ними. Пусть даже выбор будет не совпадать с моим, это не будет минусом.

M>>А зачем model и core разделили? Тем более, что core зависит от model а не наоборот. Кстати, а зачем в "реальном проекте" разделение делать иначе? Какие фундаментальные причины этого требуют?
A>чтобы model отдать клиенту, core ему не нужен (либо перенести ifaces из core в model, кто тоже стоит отдельного обсуждения)
Еще один хороший комментарий! Можно было бы про него в readme упоминуть. В том числе и потому, что решение по-умолчанию — странное. Для публичных API у вас клиенты будут не только на java. Им ваша модель вообще никак не поможет. И даже на Java — вы ведь не считаете всех остальных разработчиков полными идиотами? (Или все-же считаете?) Я думаю, они смогут сами написать модель, которая им нужна (с нужной им декомпозицией, используемыми полями и т.п.). Может, они даже возьмут десериализатор без издержек и им будут непонятны ваши компромиссы.

Да, стоит позаботиться о программистах с ограниченными возможностями навыками. Для них можно сделать отдельную библиотеку с моделями и даже API клиентом. Это будет внешняя по отношению к серверу библиотека. Или несколько библиотек на разных языках. Немного больше повторяющегося кода, но нет лишней связности меджу реализацией API и клиентом. Простое, понятное и скучное решение. А вот попытка сделать общую модель между сервером и клиентом — это как раз overarchitected.

M>>Вот весь тот список — это "элементы стиля". Вещи, которые делаются вообще автоматически. Показывает, насколько разработчик механически следует шаблонам и насколько все же задумывается о причинах определенных технических решений.
A>большинсто про вкус, остальное minor по мне. нейминг — не технические решения, как и работа с Assert
Ну... Вкус, да. Вы ведь понимаете, что код больше читается, чем пишется? Поэтому мне очень интересен код, который легко читать. В котором не нужно задумываться, что же хотел нам сказать автор. В котором можно зайти в документацию и прояснить непонятные детали. Иначе через три месяца другой разработчик поймет ваш код немного не так, как он задумывался, и в системе возникнет плавающий баг. Стиль — это эвристики, учитывающие особенности работы человеческого мозга. Например, разворачивание глубокой вложенности в последовательный код — это workaround на ограниченность кратковременной памяти. Большинство обычных людей забудет к третьему вложенному if, что там было на верхнем уровне. А потом сделают ошибку или потратят дополнительное время на вспоминание.

Я несколько лет назад осознал, что "технические решения" — это только половина программирования. А вторая половина — гуманитарные науки, литература в первую очередь. Те самые нелюбимые технарями сочинения — это суть написания кода. Точное описание своих мыслей не только для машины, но и для программистов, читающих и сопровождающих за вами ваш код. Понимание того, что ваши мысли, ваше изложение и то, что понял читатель — это три разные вещи. Умение встать на сторону читателя и писать так, чтобы он увидел то, что вы хотели сказать.

Вот все эти Assert — это как раз сложности выражать свои мысли в прямом и явном виде. Не технические, ну и что? Мне не нужны ребусы в коде. Мне нужен программист, который не поленился прочитать API и выбрать наиболее выразительный метод (для тестирования они еще и сообщения об ошибках лучше создают). Это один из многих факторов, на которые я обращаю внимание. И всегда очень интересно, когда на ревью кода кто-то другой показывает мне более выразительный (простой и понятный) вариант, чем я использовал.