Домашнее задание — Code Review

Здравствуйте! Обычно, я игнорю вакансии с домашними заданиями, но тут задача выглядела достаточно интересно и не тривиально, так что я решил выполнить в качестве некоего вызова самому себе. В задании было две задачи: по питону и по SQL, рекрутер говорила, что остальные кандидаты выполняют его за ~4 часа. Честно говоря, с чистым SQL дел имел очень мало, в основном вся работа идёт через ORM с промежуточным сохранением данных, так что думал, что основные проблемы будут именно там.

На удивление SQL я сделал часа за 3-4, но пришлось конечно подчитать доки по Postgres’у. А вот с питоном всё вышло не так гладко, конечное решение у меня заняло около 10-12 часов (можно сделать небольшую скидку на то, что делал я это по вечерам с перерывами, но все-равно считаю, что достаточно долго). Задание выполнил в срок и как договаривались отправил рекрутеру zip-архив с решениями. Мне было очень интересно, как они его оценят, какие минусы найдут, ну и в идеале покажут эталонное решение (моё мне не очень нравится, чувствую, что можно проще, но как — не придумаю).

Огромным разочарованием был ответ:

"""

Thanks for taking the time to send us your CV. Unfortunately, this time we won’t be moving forward with your application.
While we aren’t able to offer personal feedback, it helped us evaluate whether this role would be the right one for you, your profile, and *company name*.

"«"

В первую очередь очень обидно за отсутствие вообще какого-либо фидбека, ну и это конечно показывает огромный непрофессионализм данной конторы (иначе уже их назвать не могу). Для меня собеседования — это также и некие точки роста, возможность послушать умных людей и что-то у них перенять. Мне бы очень хотелось получить какой-то фидбек по заданию, поэтому буду очень благодарен за код-ревью со стороны DOU-сообщества ) Если кому не сложно — посмотрите плз, что можно было сделать лучше. Спасибо!

Ссылка на гитхаб с задачами и решением.

P.S. Для себя теперь принял за правило: если и буду делать тестовое задание, то только с предварительной отправкой мне зашифрованого файлика с ответами.

UPD_1: Силами небезраличных пользователей @Yuriy Znovyak, @Sergey Boryssenko и конечно же самого активного @Олексій Пєніє правильно было подмечено, что я забыл сгруппировать полученные результаты по user_id. Тут только могу согласиться с @Олексій Пєніє и тем, что «текст по-дыбильному написан» — я почему-то был уверен, что нужна такая же история транзакций пользователя, только в GBP.

UPD_2: @Yuriy Znovyak набросал более простой вариант решения таска на питоне, за что ему большое спасибо.

В принципе, адекватную критику и способы улучшения по своей домашке я получил, мозг успокоился ) Спасибо вам всем, ребят, помогли!

👍НравитсяПонравилось1
В избранноеВ избранном0
LinkedIn
Допустимые теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter
Допустимые теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter

Повністю згоден, що відсутність фідбека по тестовому — це злочин перед кандидатом (якщо тільки вас не взяли).
Наступного разу раджу уточнювати, чи буде фідбек. У вашому випадку цілком могло бути, що ТЗ ніхто і не дивився.

Яка назва компанії?

Хорошая тема — лично для себя окончательно решил игнорить тестовые.
Первое — мотивация на ноле, делаешь без души;
Второе — приоритеты явно не в эту сторону;
Третье — feedback абсолютно не гарантирован, даже если обещают.
Последнее — «без качественного ТЗ, результат ХЗ», и это как правило сплошь и рядом.

Якщо воно займає не більше години — чому б ні? Або якщо його добре оплачують

рекрутер говорила, что остальные кандидаты выполняют его за ~4 часа
решение у меня заняло около 10-12 часов

Это всегда так. Если говорят про «4 часа», то готовься все выходные просидеть.

Огромным разочарованием был ответ:

Жене одна контора тоже так ответила на ТЗ. Я сразу смекнул, что это происки ушлого рекрутера, которому просто лень/страшно дёргать инженера для фидбека. Написали ему пассивно-агрессивный ответ и фидбек пришёл вместе с извинениями от лида. Потом лид ещё лично в LinkedIn писал с извинениями и более развёрнутым фидбеком. Но это был довольно известный fintech в Лондоне, поэтому им было что терять в репутационном плане. Если у вас что-то подобное, то есть смысл надавить. Но если вы имеете дело с очередной аутсорсинг-галерой, то нет даже смысла в диалог вступать.

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

З пітоном ти щось перемудрив.

По-перше ти генеруєш неправильний json, наприклад якщо твою прожку прогнати, то буде:

$ cat input.json | python3 nest.py currency country city
{'EUR': {'ES': {'Madrid': [{'amount': 8.9}]},
         'FR': {'Lyon': [{'amount': 11.4}], 'Paris': [{'amount': 20}]}},
 'FBP': {'UK': {'London': [{'amount': 10.9}]}},
 'GBP': {'UK': {'London': [{'amount': 12.2}]}},
 'USD': {'US': {'Boston': [{'amount': 100}]}}}

Це неправильний json, бо повинно бути ", а не '. pprint чисто для друкування пітонових словників. Можеш скопіпастити в jsonformatter.org і побачиш, що він ругнеться. Правильно було б:

    print(json.dumps(create_group(arr, args.group_keys), indent='  ')
або (чуток правильніше, але зайобистіше):
    json.dump(create_group(arr, args.group_keys), sys.stdout, indent='  ')

А чому

parser.parse_args(sys.argv[1:])
а не просто:
args = parser.parse_args()

Правильно було б:

    json_arr = sys.stdin.read()
а не
    json_arr = ''.join(line.strip() for line in sys.stdin)
але ще краще було б напряму з sys.stdin загрузити json:
    arr = json.load(sys.stdin)

Я швиденько таке наговнякав:

#!/usr/bin/env python3
import argparse, json, sys


def main():
    parser = argparse.ArgumentParser(
        description="Group list of dicts according to input group keys",
    )
    parser.add_argument('group_keys', nargs='+', help='List of group keys')
    args = parser.parse_args()
    arr = json.load(sys.stdin)
    json.dump(create_group(arr, args.group_keys), sys.stdout, indent='  ')


def create_group(arr, keys):
    if not keys:
        return arr

    root = dict()
    for elem in arr:
        pos = root
        for key in keys:
            k = elem[key]
            if k not in pos:
                pos[k] = dict() if key != keys[-1] else list()
            pos = pos[k]

        pos.append({k: v for k, v in elem.items() if k not in keys})
    return root


if __name__ == '__main__':
    main()

Был уверен, что делаю через чур сложно, но проще вариант чет не пришел в голову. Огромное спасибо за ваше ревью, классная идея с разделением ключей на те, которые пришли из инпута и остальные. Я всё пытался сначала через рекурсию решить, но тут немного другой случай. Чтение из stdin у вас тоже получше, согласен. В принципе, этот то, чего мне и не хватило после собеса, еще раз спасибо огромное.

Та взагалі без проблем. Насправді це тобі мега-респект за те, що не побоявся запостити код, бо тут спільнота доволі таки токсична (і в цьому є свій шарм) і в 99% випадків закидає гівном (навіть якщо все ок). Але це така собі місцева специфіка і якщо відкинути хейтерів, то в гіршому випадку ти просто не дізнаєшся нічого нового і втратиш чуток часу. А от upside, теоретично, взагалі не обмежений зверху.

бо тут спільнота доволі таки токсична

Особливо якщо прийти сюди зі спамом чи за ДіяСісю топити

і в 99% випадків закидає гівном

і в 100% вашим власним. Спробуйте принести щось краще :)

Звісно, є такі, хто просто тролі. Але таких зазвичай навіть просити не треба обхезатись, вони вже прийшли з відром дуже старого «перевіреного» контенту, який окрім сміху нічого не викликає.

Блин коллеги, не делайте эти ТЗ вообще.
Если дают ТЗ, значит новый разработчик не сильно то и нужен.
Когда разработчик нужен на вчера, оффер будут давать через 20 минут после захода в офис, и потом месяц уговаривать его принять.

оффер будут давать через 20 минут после захода в офис

Как будто это что-то хорошее...

Нет, лучше все выходные делать ТЗ и не получить никакой фидбек.

Это вторая крайность. Можно (нужно) заранее уточнять что собой представляет ТЗ и вежливо отказаться, если оно требует слишком много времени. Но если на входе нет никакого фильтра вообще, то с вероятностью овер 99% разгребать там придется такое говнище что ну его нах...

Мне на 95+% позиций не давали вообще никакого ТЗ.
Если ТЗ пытаются дать, то значит в тебе изначально не видят синьора, если бы видели, то не давали были.
Как мидлом был, тоже больше половины позиций были без ТЗ.

Согласен с вами. Я у них уточнил — сказали около 4 часов, что для меня был норм, но в реальности оказалось дольше (

Но надо ж было ещё и решить правильно. А тест — это прежде всего для тебя. Потому что если ты сам такое не решаешь вовремя, то тебя вышвырнут на испытательном сроке. Логично тебя пока не брать совсем и ни в какие блек-листы не вносить.

Да там достаточно простое ТЗ... было бы, если бы не занимались дуристикой и корректно ставили условия. Там же маразм из серии «а догадайся, что мы имели в виду». Почему и предположил, что эти задания тупо взяли на помойке литкод или аналогичной свалке.

Ты так говоришь как будто бывают нормальные ТЗ.
Все ТЗ в жанре: «Пойми что у нас в голове».

Я завдання по пітноу не дивився, але по SQL там досить адекватно попросили чувака написати аж 1 SQL запит на JOIN і на GROUP BY. Завдання № 2 і № 3 вже були тіпа для «крутих», а Діма чесно признався, що Postgres вперше в очі побачив (це абсолютно ок). По-хорошому цей запит могли попросити написати прямо на співбесіді, але тут би народ нив, що «єбать валять» і це не реальна робота, бо в реальній роботі народ гуглить.

Єдина претензія до того завдання це те, що воно трохи фігово сформульовано, але це досить легко фікситься в наступних ітераціях. Ну тобто можна було б сказати який expected output ти хочеш і зразу відпало б 95% запитань.

І ще раз — там не якийсь охуєвший об’єм роботи просять, а тупо один (або два або три) запити на ~10 рядків.

Якби ще інглішем володіли (чи хоча б логікою), та коли копіпастять чужі тестові завдання — не намагалися їх перекладати. Там більша проблема зрозуміти, чого саме вони хочуть. А завдання саме є дійсно простим тестом. І автор його провалив.

Таке трапляється. На те вони й тести. Але до написання умов тестів треба підходити з усією логічністю, не допускаючи моментів «а здогадайтеся чого ми хочемо». Бо на відміну від справжніх ТЗ, немає іншої інформації, що дозволяє відсіяти невірні припущення. Тут чистий сурогат, мають право запитати що завгодно.

Ну не до такой же степени!! Это ТЕСТОВЫЕ задания. Суррогатные. То есть никаких других факторов, чтобы увидеть желаемое, нет в принципе. И понять, какой результат хотят получить, можно лишь НАГУГЛИВ, откуда украли это тестовое задание, и почитать полные условия. Ну и решение заодно, чего уж стесняться, если задание по сути свелось к «найди у кого мы украли».

Бля, важко повірити, але я, по-ходу, згодний з Пєнієм — умови задачі SQL трохи по-дебільному написані, але, в принципі, можна розшифрувати що хотіли.

По SQL: тебе попросили

query that gives us a breakdown of spend in GBP by each user

тобто від тебе хотіли табличку, де буде 2 стовпця userid, total_spend_gbp. Причому очікується, що ти будеш агрегувати по userid, а у тебе на вхідних даних 4 рядка для userid=1. В теорії можна пофіксити так:

SELECT transactions.user_id,
        SUM(transactions.amount * COALESCE(actual_exchange_rate.rate, 1)) as total_spent_gbp -- 99%, що хотіли SUM()
    FROM transactions
    LEFT OUTER JOIN ( -- досить неортодоксально, але наче ок
            SELECT DISTINCT ON (from_currency) from_currency, rate
            FROM exchange_rates
            WHERE to_currency = 'GBP'
            ORDER BY from_currency, ts DESC
    ) as actual_exchange_rate ON actual_exchange_rate.from_currency = transactions.currency
    GROUP BY 1 -- хотіли ось це
    ORDER BY transactions.user_id;

Насправді далі вже діло смаку, але такі запити досить прикольно писати в 2 кроки:

select userid, sum(spend_gbp) from (
   -- тут приблизно те, що ти зробив  
) group by 1 order by 1;
або за допомогою CTE (www.sql-tutorial.ru/...​able_expressions_cte.html) — щоб легше було розшифрувати що там за ***ня відбувається.

№ 2 і № 3 ще не дійшли руки зробити.

А що таке «a breakdown of spend» — можеш пояснити? Наприклад, я не знаю цього терміну (вірніше знаю, але він може трактуватися по різному, бо не є сталим юридичним поняттям). Навіть по рішенню не зрозумів, що воно таке, чого саме хотіли. Тупо відсортувати та просумувати?

До речі, зі складності рішення йде загальне правило: не робити на базі даних те, що можна зробити в коді, якщо кількість даних до передачі не є космічною. Бо в коді все на порядок-два зрозуміліше, і на стільки ж нижча ймовірність помилок.

SQL гарний саме для того, для чого він створений. А от там де йде логіка — код має насамперед читатися. Не втрачаючи швидкості. Швидкість передачі даних до бекенду зазвичай вища, ніж створення усяких тимчасових таблиць, якщо кількість даних — декілька кілобайт. Але ж для цього треба не зловживати прокладками до SQL. Їх якраз варто використати, передавши їм готові ключі записів, за які вони можуть ліниво витягувати дані.

Для таких простих задач це звісно пофіг. Але щойно таблиці матимуть іншу кількість записів (реальну) — ото вже щастя буде, як тягнути все. Особливо коли там пагінація додасться — ото реально база просяде.

А що таке «a breakdown of spend» — можеш пояснити?

Можу. Breakdown дуже часто використовують як «розкласти на складові частини». На які саме частини?

by each user

З dictionary.cambridge.org/...​tionary/english/breakdown

a division of information into parts that belong together:
We need a breakdown of the statistics into age groups.
a division of something into its parts, so that you can see all the details:
I asked for a full breakdown of the costs involved in setting up a new website.
Навіть по рішенню не зрозумів, що воно таке, чого саме хотіли. Тупо відсортувати та просумувати?

Тебе попросили порахувати суму транзакцій (витрат) для кожного користувача, але також попросили сконвертувати з валюти транзакції в GBP. В простій (першій) задачі курс обміну це останній по часу запис курсу валют (тобто якщо є записи про курс USD->GBP за 8 ранку і 9 ранку, то візьми той, що за 9 для всіх транзакцій, навіть тих, що за 8:30). В складній (другій) задачі просять все-таки врахувати який був курс на момент часу здійснення транзакції: тобто для транзакції о 8:30 треба взяти курс за 8 ранку, а для транзакції о 9:45 треба взяти курс за 9 ранку.

До речі, зі складності рішення йде загальне правило: не робити на базі даних те, що можна зробити в коді, якщо кількість даних до передачі не є космічною. Бо в коді все на порядок-два зрозуміліше, і на стільки ж нижча ймовірність помилок.

Задача дуже правдоподібна. Дуже часто у тебе є записи про різні експерименти і тобі треба ретроактивно порахувати чи був якийсь користувач під впливом речовин експерименту. Якби ми тоді могли це зберігати, то було б просто афігенно, але ми не додумались тоді це робити, тому прийдеться писати код, щоб зрозуміти який був стан речей на той-то момент. SQL для таких цілей це просто ідеальний інструмент.

Слово «breakdown» насамперед має значення «катастрофа», «необратимий крах». І «breakdown of spend» для носія мови — це зовсім не аналітичне групування, а виявити, коли і де стався обрив трат у неконтрольоване падіння. Breakdown — це дослівно руйнування. Тому й кажу, треба коректно ставити умови.

Те саме стосується умовою по валюті: для автора синтетичної задачі трати в різних валютах — це «виніпанімаєтє, етодругоє». А для тих, хто розуміє суть транзакцій, це все дані однієї природи, в різних одиницях виміру. Тому я б намагався зрозуміти, як саме очікується зробити конверсію валют. І тому що завдання тестове, тупо створив би таблицю курсів. Мене б відсіяли, тому що на виході тесту «нєправільний отвєт». В той час, коли це тупо некоректно поставлена задача.

Задача дійсно фігово сформульована і я з тобою тут погоджуюсь. Але йопт, давай також не робити вигляд, що там була якась Енігма, для якої треба найкращим криптографам NSA працювати 24/7, щоб зрозуміти чого хотів автор.

Ну і придираєшся до слів ти теж зря, бо ти чудово знайєш, що є дофіга слів, які можуть мати однакове написання і вимову, але означати різні речі — омоніми (homonym). В тому кембріджському словнику, який я рандомом нагуглив, обидва значення є.

Тестове завдання, для якого потрібен кембріджський словник — вірна ознака що людині не можна дозволяти писати тестові завдання та взагалі ТЗ. Банальна некомпетентність. Звільнити нафіг — ото правильний варіант. Теж мені, проблема сторіччя, знайти людей, які вміють працювати з людьми та володіють елементарними мовними навичками.

До речі, словник не допоможе, коли підходять одночасно кілька значень, а використали якесь не дуже поширене, тоді як просте і зрозуміле — підходить. Інакше кажучи, коли з точки зору носія мови написане лайно. Не вмієте в мову — та зверніться ж до тих, хто вміє. Невже на весь департамент жодного флуент носія? Тупо перевірити коректність написання та й все. Подумаєш, дрібничка — профукати ринок та переплачувати за кадри чималі кошти, замість того щоб просто грамотно оформити тестові завдання.

Спасибо! Согласен, что провтыкал group_by, но я чет подумал, что надо оставить такой же журнал, только в GBP.

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

1	3.9690
1	13.6890
1	2.170
1	3.9690
2	1.5190
2	0.2790
2	20.770
2	20.1690
3	2
4	1.24
4	2
а должно быть
1	23.797
2	42.737
3   2
4	3.24

Ну, в принципе да. Если ТС понял условие, но неправильно решил — с ним не о чем говорить, ибо время на проверку у него было.

Полностью согласен, group_by я как-то и не заметил... Я неправильно понял условия, мне казалось, что надо сделать такую же историю, но в GBP... Тут мой косяк, по-любому, — не дочитал.

Глянул одним глазом задачи по SQL. Как и ожидалось, некорректные условия — наше всё. Решить не проблема, а вот понять чё хотят — берите чётки и хрустальный шар. Вы чё их, с литкода спёрли? Тогда почему не спёрли пояснения к условию, или «сами должны догадаться»?

За название таблицы «transactions» — по лапкам бы надавать линейкой. Давайте уж все ключевые слова задействуйте. За таблицы без ключей, тем более в тестовом задании — дать выбор на 2 стула. За задание таймстемпа через дату без указания часового пояса — платить зарплату только по 31 числам.

Если кратко, люди, которые не имели дела с реальными задачами — не имеют право тестить.
Отдельно заплакал с условия задачи «для гуру», которая должна выполняться не дольше 5 секунд... для десятка записей. А сотни миллионов записей в реальных задачах — не хотели?? Вы реально готовы ждать 3 года, пока запрос исполнится? Ну гуру же. Можно я сразу ответ скажу: 42

Спасибо, что посмотрели. Я их не откуда не спёр — это то, что прислал рекрутер. В файле ’SQL task’ точная копипаста из оригинального файла, больше ничего не было. В целом задача: взять данные о тратах юзера и посчитать их в GBP согласно таблице курсов валют

Я ж и говорю, рекрутер спёр, или тот кто на самом деле технический собес проводил.

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

А в чем ценность ключей в данном случае?

Якщо у тебе в таблиці транзакцій N рядків, а в таблиці курсів обміну M рядків, то без індексів у тебе час роботи буде O(NM), тобто довго. Якщо на таблицю обміну курсу навісити індекс (b-tree) по (to_currency, from_currency, ts), то look up по останньому курсу xyz->gbp ДО часу ts, по ідеї, можна буде зробити за O(log M), тобто сумарний час роботи буде O(N log M) — в принципі повинно влізти в time limit.

Індекс по (from_currency, to_currency, ts) здається теж повинен спрацювати. А от (ts, from_currency, to_currency) і (ts, to_currency, from_currency) вже не спрацює.

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

Если цель чисто выполнить на время — то подключил бы помощь, как сделал 1 другой кандидат. Если цель просто выполнить — то пошли они нахер, надо было оговаривать срок. Задание-то бесплатное. А если это реальная боевая задача, которую типа как тест дали — то тем более пошли в задницу.

Грубо говоря, очередные HR-сектанты не умеют ничего в своей профессии. И 16-летние синьоры им под стать.

Так по времени я успел, всё норм. А тестовое выполнял больше для себя: проверить насколько где заржавело в мозгах )

Нинасколько. Это маразм, а не задачи. Они на то, чтобы угадать «что имелось в виду». И вероятнее всего их цель — пропустить заранее известного кандидата, и показательно отсеять остальных.

Хочешь туда зайти — найди другого рекрутера той же конторы и пройди как дети в школу. Поверь, правая рука не знает, что делает левая.

Та уже не особо и хочу ) Куча других, более адекватных вакансий.

Да ты сначала займи одну. А то ведь тест-то провалил в том числе сам.

возможность послушать умных людей

Иногда умных людей нету и слушать нет кого.
Пару раз брал тестовое но так и не успевал сделать, получал оффер от других контор, да и при работа+поиски новой работы времени в обрез, а с тестовым надо с умомо обычно заниматься, а не в перерывах между митингами. С тех пор отказываюсь от тестового.

Подписаться на комментарии