Что не так с тестовым заданием? - О работе

Здравствуйте, 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 и выбрать наиболее выразительный метод (для тестирования они еще и сообщения об ошибках лучше создают). Это один из многих факторов, на которые я обращаю внимание. И всегда очень интересно, когда на ревью кода кто-то другой показывает мне более выразительный (простой и понятный) вариант, чем я использовал.

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

Вам не кажется что с таким отношением можно примелькаться до лисчной неприязни и попадания в бан-листы? Т.е. потенциальным коллегам говорить заранее чтобы они потратили на Вас уйму времени чтобы Вы "потренировались"?

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

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

. И в третьих, контор очень много. Более одного раза я только в люксофт собеседовался, наверно раз 5

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

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

L>>Зачем вообще нужен add, когда у вас в задании "money transfers between accounts"?
A>потому что они как-то на аккаунтах должны появляться
Деньги не могут появиться на счете из воздуха, у них всегда есть источник — парный счет в транзакции.

Т.е. если бы я был сотрудником этой конторы, проводящем ревью тестового задания, то в заключении бы написал, что, скорее всего, очень много придется вложиться в обучение и погружение в предметную область и первые полгода-год кандидату нельзя будет поручать design & architecture.

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

A>The solution is no concurrency safe — transaction is not atomic
A> me — there is no explicit requirement and actually basic
A>transaction (add) is atomic. There is no race conditions. Atomicity of
A>_transfer_ is required if we don't want to loose data — but here there
A>is no such problem since we don;t store anything between sessions.
Они, конечно же, правы. Комон сенс здесь как бы орет, что метод должен быть трэдсейф.

A>Insufficient testing coverage
A>nothing to say. It's 6hrs test. Not 40hrs
Ты в курсе, что твои комменты очень грубые? Люди платят премиальные деньги не только за скилл, но и за приятную работу. Ты конечно не послушаешь, но я бы посоветовал тебе поработать над софт скиллами.

A>If we transfer negative amount money, author don’t throw exceptions, it takes abs value of this amount and transfer positive amount of money.
A>transferring negative amount is something which is odd. So it's a
A>convention. It either has to be documented or exception should be
A>thrown. So I partially agree.
ИМО это эпичный фэйл, а не partially agree.

A>Author don't know how to catch exceptions in junit
A>minor. Nothing to add.
Эм. Ты и правда не знаешь как ловить эксепшены или это что-то другое?

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

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

A>>The solution is no concurrency safe — transaction is not atomic
A>> me — there is no explicit requirement and actually basic
A>>transaction (add) is atomic. There is no race conditions. Atomicity of
A>>_transfer_ is required if we don't want to loose data — but here there
A>>is no such problem since we don;t store anything between sessions.
B>Они, конечно же, правы. Комон сенс здесь как бы орет, что метод должен быть трэдсейф.

он thread-safe во всех смыслах.

A>>Insufficient testing coverage
A>>nothing to say. It's 6hrs test. Not 40hrs
B>Ты в курсе, что твои комменты очень грубые? Люди платят премиальные деньги не только за скилл, но и за приятную работу. Ты конечно не послушаешь, но я бы посоветовал тебе поработать над софт скиллами.

кто платит премиальные деньги? откуда такая информация? и откуда ты знаешь насколько со мной приятно работать?

A>>If we transfer negative amount money, author don’t throw exceptions, it takes abs value of this amount and transfer positive amount of money.
A>>transferring negative amount is something which is odd. So it's a
A>>convention. It either has to be documented or exception should be
A>>thrown. So I partially agree.
B>ИМО это эпичный фэйл, а не partially agree.

эпичный фейл это отсутствие abs. а с abs это просто другая культура

A>>Author don't know how to catch exceptions in junit
A>>minor. Nothing to add.
B>Эм. Ты и правда не знаешь как ловить эксепшены или это что-то другое?

эмм, ты и правда доволен своими софт-скилами?

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

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

A>>напоминает простой кошелек. Нейминг не на все 100 удачный, возможно.
L>В том-то и дело, что кошелек принадлежит конкретному владельцу, у вас же через него проходят транзакции кого угодно. В этом и главная проблема выбранного имени.

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

L>>>Зачем вообще нужен add, когда у вас в задании "money transfers between accounts"?
A>>потому что они как-то на аккаунтах должны появляться
L>Деньги не могут появиться на счете из воздуха, у них всегда есть источник — парный счет в транзакции.

когда ты приносишь кеш в банк тоже есть парный счет? здравый смысл подсказывает, что в контексте этого тестового задания они обязаны появляться из воздуха

L>Т.е. если бы я был сотрудником этой конторы, проводящем ревью тестового задания, то в заключении бы написал, что, скорее всего, очень много придется вложиться в обучение и погружение в предметную область и первые полгода-год кандидату нельзя будет поручать design & architecture.

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

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

A>>>Insufficient testing coverage
A>>>nothing to say. It's 6hrs test. Not 40hrs
B>>Ты в курсе, что твои комменты очень грубые? Люди платят премиальные деньги не только за скилл, но и за приятную работу. Ты конечно не послушаешь, но я бы посоветовал тебе поработать над софт скиллами.
A>кто платит премиальные деньги? откуда такая информация?
Очевидно, что соглашаясь на тестовое задание в 6 часов, ты рассчитываешь на какие-то премиальные деньги. Ты же не станешь делать тестовое задание в контору, где платят в 2 раза меньше чем ты имеешь сейчас.

A>и откуда ты знаешь насколько со мной приятно работать?
Из фрагмента твоей деловой переписки. После такого матраса я бы на месте интервьюера порекомендовал бы no-go. Собственно, советую не очень удивляться, получая отлупы на казалось бы ровном месте. Одно дело желание агрессивно заэнфорсить свое мнение в срачике на РСДН, и совсем другое — работая в западной конторе. Впрочем, я думаю в Нетфликсе тебе бы очень понравилось.

A>>>If we transfer negative amount money, author don’t throw exceptions, it takes abs value of this amount and transfer positive amount of money.
A>>>transferring negative amount is something which is odd. So it's a
A>>>convention. It either has to be documented or exception should be
A>>>thrown. So I partially agree.
B>>ИМО это эпичный фэйл, а не partially agree.
A>эпичный фейл это отсутствие abs. а с abs это просто другая культура
Мде.

A>>>Author don't know how to catch exceptions in junit
A>>>minor. Nothing to add.
B>>Эм. Ты и правда не знаешь как ловить эксепшены или это что-то другое?
A>эмм, ты и правда доволен своими софт-скилами?
Нет, не очень, честно признаться.

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

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

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

реально не поленюсь

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

давайте без демагогии. gradle это не "еще одна билд система из миллиона билд систем". если ты видишь build.gradle в проекте и все еще не знаешь куда смотреть чтобы его собрать — ну тогда у нас с тобой разная культура и нам не по пути

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

спроси меня прямо, вместо того, чтобы додумывать. попахивает демагогией. можно еще про порно на псевдографике в readme подумать. все зависит от того кто как думать привык

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

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

M>>>Не StorageTest а StorageImplTest. Вот представьте, у вас есть несколько реализаций Storage. Какую из них тестирует StorageTest?
A>>вы серьезно?
M>Конечно. 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>>кому хотелось и зачем?
M>Ну в первую очередь вам. Потому что вы хотели продолжения интервью или найма. Хотелось бы мне. Как минимум потому, что приятно поговорить с кандидатом с широким кругозором. За реализацию Rest Level 1 вы бы получили плюсик. А на самом деле при создании API вы скорее всего вспомнили бы причины, по которым REST был спроектирован в его виде. И задумались бы про идемпотентность (idempotency) операций. Не обязательно ее реализовывать, можно было просто упомянуть в readme: "идемпотентность не реализована из-за ограничений на время, могу прислать план реализации по запросу". И получили бы сразу пять плюсиков.

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

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

. Это — основная причина, по которой Rest Level 1 хотелось бы видеть лично мне.

вот про Idempotency в тему, наконец-то. это интересный топик поговорить, но не в рамках тестового задания. остальные замечания по поводу level'ов — странная любовь к классификациям. все, что идет после verbs вообще довольно мутная история.

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

ну да, не написал, то джуниор, написал — архитект. ладно, спишем на разную культуру

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

Impl — вполне осмысленно, если нет ничего иного, и, опять же, в контексте тестового задания

M>>>Я бы вообще эти реализации спрятал за Factory Method. Потому что тогда их можно по желанию менять, разбивать/переразбивать на разные классы и т.п. В конце концов, все использования идут только через реализованные интерфейсы.
A>>нечего добавить. не вижу в 2019 году задачи явно показывать знание Factory Method
M>А вот и не угадали

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

соразмерность решения и задачи. все что могу сказать. про рефакторинги можно отдельно поговорить на интервью

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

давайте не будем заниматься гаданием что к чему приводит. тут вложенность 3. 3, не 10. "сегодня он слугает джаз..." ага

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

ждете от меня? где? в readme? нет уж

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

во-первых, я по-умолчанию буду думать, что клиенты на джава если явно не сформулировано иное — yagni
во-вторых, писать в readme то, что лучше обсудить — бред и трата времени
в-третьих — зачем писать что-то второй раз, если это уже написано? чтобы добавить багов? или чтобы притащить только ограниченный набор зависимостей? так вот эти все вопросы и нюансы использования обговариваются на собеседовании, но не пишется сочинение в readme

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

боже, хватит всех считать за идиотов.

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

а, ну знакомые слова — "все идиоты, пиши проще". прости чувак, но есть код который реально сложно и читать и писать. и поверь, мои 3 вложенных when не из их числа.
не в состоянии следить за простым контекстом? ну что могу сказать, се ля ви.

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

ты смотришь в верную сторону, только не литература — философия.
программирование — это про раскрытие сути вещей. про понятия и их связи, рациональность, полноту, именование и тп.
и тут как раз твои претензии по именованию имели бы большое значение, будь это код из prod.
имена важны, но есть еще контекст.
в моем контексте на передний план выходит вопрос достаточности и соразмерности. так вот *Impl по-моему соразмерен заданию и органичен тут, но не факт что
был бы нормален в настоящем большом проекте. если здесь я пишу *Impl, это не значит что я везде пишу Impl. Как об этом догадаться — спросить на интервью

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

ты делаешь ложные выводы из ложных предположений. это не сложности, это сознательный выбор в контексте задачи.

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

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

A>>>>Insufficient testing coverage
A>>>>nothing to say. It's 6hrs test. Not 40hrs
B>>>Ты в курсе, что твои комменты очень грубые? Люди платят премиальные деньги не только за скилл, но и за приятную работу. Ты конечно не послушаешь, но я бы посоветовал тебе поработать над софт скиллами.
A>>кто платит премиальные деньги? откуда такая информация?
B>Очевидно, что соглашаясь на тестовое задание в 6 часов, ты рассчитываешь на какие-то премиальные деньги. Ты же не станешь делать тестовое задание в контору, где платят в 2 раза меньше чем ты имеешь сейчас.

почему какая-то бинарность — либо в 2 раза меньше, либо премиальные? просто нормальные деньги. меньше, чем на моей предыдущей работе

A>>и откуда ты знаешь насколько со мной приятно работать?
B>Из фрагмента твоей деловой переписки. После такого матраса я бы на месте интервьюера порекомендовал бы no-go. Собственно, советую не очень удивляться, получая отлупы на казалось бы ровном месте. Одно дело желание агрессивно заэнфорсить свое мнение в срачике на РСДН, и совсем другое — работая в западной конторе. Впрочем, я думаю в Нетфликсе тебе бы очень понравилось.

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

A>>>>If we transfer negative amount money, author don’t throw exceptions, it takes abs value of this amount and transfer positive amount of money.
A>>>>transferring negative amount is something which is odd. So it's a
A>>>>convention. It either has to be documented or exception should be
A>>>>thrown. So I partially agree.
B>>>ИМО это эпичный фэйл, а не partially agree.
A>>эпичный фейл это отсутствие abs. а с abs это просто другая культура
B>Мде.

ну да. где-то -1 — это "все хорощо", а где-то 0. но я в целом согласен, что тут лучше с exception. я не согласен с тем, что это major

A>>>>Author don't know how to catch exceptions in junit
A>>>>minor. Nothing to add.
B>>>Эм. Ты и правда не знаешь как ловить эксепшены или это что-то другое?
A>>эмм, ты и правда доволен своими софт-скилами?
B>Нет, не очень, честно признаться.

если по теме, то я не совсем понял суть предложения "Author don't know how to catch exceptions in junit". как ловить эксепшены вне Junit я, кажется, знаю)

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

Честно говоря, по американским меркам, ты им посто нахамил. Думаю, что можно списать на разницу культур, учитывая общий уровень англ. — но вообще это просто жесть.

Ксати в дискусси на форуме нет смысла особенно препираться, это тебе с работой не поможет. Твоя задача, не нас уюедить, а понять что у тебя не так. Лучше подумай почему люди тебя не понимают.

A>The solution is no concurrency safe — transaction is not atomic
A> me — there is no explicit requirement and actually basic
A>transaction (add) is atomic. There is no race conditions. Atomicity of
A>_transfer_ is required if we don't want to loose data — but here there
A>is no such problem since we don;t store anything between sessions.

Не поленился — посмотрел, но синхронизации я не нашел. Поясни, как она делается, и это самое атомисити достигается.
Транзакии/аккаунт — классические пример для синхронизации. И да, для сервиса — это и так понятное требование.

A>Insufficient testing coverage
A>nothing to say. It's 6hrs test. Not 40hrs
в принципе тут просто более внятно надо ответить было, и все
хотя я бы тесты вместе с интерфесами нахерачил, а потом имплементацию лепил

A>If we transfer negative amount money, author don’t throw exceptions, it takes abs value of this amount and transfer positive amount of money.
A>transferring negative amount is something which is odd. So it's a
A>convention. It either has to be documented or exception should be
A>thrown. So I partially agree.
ну это вообще саботаж какой-то, кмк

A>Author don't know how to catch exceptions in junit
A>minor. Nothing to add.
мдааа

A>Procedure of transferring money is not clear and difficult to support
A>no comment.
Сложность — понятие нескольно субъективное, но да. Пишешь не очень понятно местами. Кучка разбрсанных мелких классов,

A>Poor documentation — there is no specifications of endpoints, how to build a project
A>KL — there is a README. And there is a build.gradle. It's kind of
A>misunderstanding what we expect from 6hrs test.

Вот кстати, уже более внятное объяснение. К концы начало получаться немного.

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

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

CAF>Честно говоря, по американским меркам, ты им посто нахамил. Думаю, что можно списать на разницу культур, учитывая общий уровень англ. — но вообще это просто жесть.

а при чем здесь америка? может они из австралии? а по факту русские пацаны из питера

CAF>Ксати в дискусси на форуме нет смысла особенно препираться, это тебе с работой не поможет. Твоя задача, не нас уюедить, а понять что у тебя не так. Лучше подумай почему люди тебя не понимают.

откуда ты знаешь какая у меня задача? может мне тут пободаться в кайф и работа мне вообще не нужна?

A>>The solution is no concurrency safe — transaction is not atomic
A>> me — there is no explicit requirement and actually basic
A>>transaction (add) is atomic. There is no race conditions. Atomicity of
A>>_transfer_ is required if we don't want to loose data — but here there
A>>is no such problem since we don;t store anything between sessions.

CAF>Не поленился — посмотрел, но синхронизации я не нашел. Поясни, как она делается, и это самое атомисити достигается.
CAF>Транзакии/аккаунт — классические пример для синхронизации. И да, для сервиса — это и так понятное требование.

ConcurrentHashMap.merge — атомарен. Строго говоря для совершения транзакции должно соблюдаться три условия:
1 атомарное дебетирование источника
2 атомарное кредитование получателя
3 предшествование первого второму

атомарность всей транзакции — необязательна. я могу сначала списать 2 суммы с одного и того же счета а уже потом перевести их на другие. тут гранулярность не на уровне
перевода со счета на счет, а на уровне изменения баланса аккаунта
единственный вопрос тут может быть к #3 из-за возможного reordering'a инструкций и отсутствия явных memory barriers. но я думаю и тут все чисто

и тут уже не один раз мешали все в кучу — атомарность, потокобезопасность, concurrency, синхронизация.
это все разные вещи.

A>>Insufficient testing coverage
A>>nothing to say. It's 6hrs test. Not 40hrs
CAF>в принципе тут просто более внятно надо ответить было, и все
CAF>хотя я бы тесты вместе с интерфесами нахерачил, а потом имплементацию лепил

A>>If we transfer negative amount money, author don’t throw exceptions, it takes abs value of this amount and transfer positive amount of money.
A>>transferring negative amount is something which is odd. So it's a
A>>convention. It either has to be documented or exception should be
A>>thrown. So I partially agree.
CAF>ну это вообще саботаж какой-то, кмк

я в ветке уже несколько раз отвечал.

A>>Author don't know how to catch exceptions in junit
A>>minor. Nothing to add.
CAF>мдааа

я вообще не очень понимаю суть претензий от них. ты вот мне можешь вместо "мдааа" написать в чем косяк?

A>>Procedure of transferring money is not clear and difficult to support
A>>no comment.
CAF>Сложность — понятие нескольно субъективное, но да. Пишешь не очень понятно местами. Кучка разбрсанных мелких классов,

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

A>>Poor documentation — there is no specifications of endpoints, how to build a project
A>>KL — there is a README. And there is a build.gradle. It's kind of
A>>misunderstanding what we expect from 6hrs test.

CAF>Вот кстати, уже более внятное объяснение. К концы начало получаться немного.

ну мне не 15 лет чтобы хотеть всем нравиться

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

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

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

CAF>>Честно говоря, по американским меркам, ты им посто нахамил. Думаю, что можно списать на разницу культур, учитывая общий уровень англ. — но вообще это просто жесть.

A>а при чем здесь америка? может они из австралии? а по факту русские пацаны из питера
Я написал для примера.
Раз уж у тебя собеседование на англ., то и культуру в конторе перенимают оттуда скорее всего.

CAF>>Ксати в дискусси на форуме нет смысла особенно препираться, это тебе с работой не поможет. Твоя задача, не нас уюедить, а понять что у тебя не так. Лучше подумай почему люди тебя не понимают.

A>откуда ты знаешь какая у меня задача? может мне тут пободаться в кайф и работа мне вообще не нужна?

Да видимо так и есть.

A>>>The solution is no concurrency safe — transaction is not atomic
A>>> me — there is no explicit requirement and actually basic
A>>>transaction (add) is atomic. There is no race conditions. Atomicity of
A>>>_transfer_ is required if we don't want to loose data — but here there
A>>>is no such problem since we don;t store anything between sessions.

CAF>>Не поленился — посмотрел, но синхронизации я не нашел. Поясни, как она делается, и это самое атомисити достигается.
CAF>>Транзакии/аккаунт — классические пример для синхронизации. И да, для сервиса — это и так понятное требование.

A>ConcurrentHashMap.merge — атомарен. Строго говоря для совершения транзакции должно соблюдаться три условия:
A>1 атомарное дебетирование источника
A>2 атомарное кредитование получателя
A>3 предшествование первого второму

A>атомарность всей транзакции — необязательна. я могу сначала списать 2 суммы с одного и того же счета а уже потом перевести их на другие. тут гранулярность не на уровне
A>перевода со счета на счет, а на уровне изменения баланса аккаунта

У тебя даже ее нет, я это имел ввиду. У тебя если 2 запроса одновременно работают с одним аккаунтом, то все может быть

Проверил хватает ли денег в первой транзакциии, затем вьорая все списала, и вот ты в минусе
Нужен лок на всю операцию по аккаунту.
A>единственный вопрос тут может быть к #3 из-за возможного reordering'a инструкций и отсутствия явных memory barriers. но я думаю и тут все чисто
Не barriers, a fences. И причем тут они?
A>и тут уже не один раз мешали все в кучу — атомарность, потокобезопасность, concurrency, синхронизация.
A>это все разные вещи.
А причем тут я?

A>>>Insufficient testing coverage
A>>>nothing to say. It's 6hrs test. Not 40hrs
CAF>>в принципе тут просто более внятно надо ответить было, и все
CAF>>хотя я бы тесты вместе с интерфесами нахерачил, а потом имплементацию лепил

A>>>If we transfer negative amount money, author don’t throw exceptions, it takes abs value of this amount and transfer positive amount of money.
A>>>transferring negative amount is something which is odd. So it's a
A>>>convention. It either has to be documented or exception should be
A>>>thrown. So I partially agree.
CAF>>ну это вообще саботаж какой-то, кмк

A>я в ветке уже несколько раз отвечал.
Я видел, но мнение высказал. Это ошибка, и очень груюая.
A>>>Author don't know how to catch exceptions in junit
A>>>minor. Nothing to add.
CAF>>мдааа

A>я вообще не очень понимаю суть претензий от них. ты вот мне можешь вместо "мдааа" написать в чем косяк?
Мдаа по поводу ответа. Вот спросил бы у них, что им не нравится.

A>>>Procedure of transferring money is not clear and difficult to support
A>>>no comment.
CAF>>Сложность — понятие нескольно субъективное, но да. Пишешь не очень понятно местами. Кучка разбрсанных мелких классов,

A>о ужас, разбросанные по именованным модулям мелкие классы. очень обще, давай конкретику

На кой она тебе? Я же говорю, не особо понятно пишеш. В маленком проекте все ок, а в большом не знаю что будет

A>>>Poor documentation — there is no specifications of endpoints, how to build a project
A>>>KL — there is a README. And there is a build.gradle. It's kind of
A>>>misunderstanding what we expect from 6hrs test.

CAF>>Вот кстати, уже более внятное объяснение. К концы начало получаться немного.

A>ну мне не 15 лет чтобы хотеть всем нравиться

Причем тут это?

	От:	vsb
	Дата:	07.03.19 07:59
	Оценка:	+1

	От:	arth
	Дата:	07.03.19 09:58
	Оценка:

	От:	elmal
	Дата:	07.03.19 17:10
	Оценка:	+2

От:	RussianFellow	http://russianfellow.livejournal.com
Дата:	07.03.19 18:58
Оценка:	-1

	От:	mgu
	Дата:	07.03.19 22:32
	Оценка:

	От:	maxkar
	Дата:	08.03.19 10:34
	Оценка:	3 (1) +3

	От:	Glestwid
	Дата:	08.03.19 18:24
	Оценка:

	От:	lintik
	Дата:	08.03.19 19:41
	Оценка:

	От:	binnom
	Дата:	08.03.19 22:11
	Оценка:	+1