Комментарии 20
Побуду хейтером, но я бы при виде такого кода начал задавать много вопросов.
Вёрстку особо трогать не буду, но что там абсолютные значения высоты - а задумывались что будет если фамилия например длинная и не влезет в одну строку?
ApiResult выглядит опасно, у вас можно быть в теории Success с null значением? Мне кажется это уже совсем не успех.
Адаптер - смесь when и if выглядит странно, плюс у вас адаптер и холдер склеены в один класс.
Куча мест где есть проверка на null выглядит странно, чисто для примера Color.parseColor(item.lesson?.color) что тут будет если цвет null?
sortLessons выглядит максимально странно, создание мутабельного map куча конвертов данных, отдача изменяемого списка.
Да ладно, все с чего-то начинают :)
Будет стараться все получится. Правда дизайн от реализации далёк конечно...
Отвечу на некоторые замечания
По верстке да, есть такой недочет длинные ФИО налезают на соседнюю View.
По поводу ApiResult, согласен, сейчас я использую другой способ обработки ответов сервера.
По поводу проверки на null, на серьезном проекте да, могут поступать разные данные, могут и с null, но в данном тестовом в ответе всегда есть данные, поэтому приложение не упадет.
sortLessons на момент написания тестового такой способ сортировки и заполнения мне казался хорошим решением.
Спасибо за то что указали на недочеты, безусловно мне есть над чем поработать, в следующих своих работах буду учитывать такие моменты. Собственно обратная связь и практика помогают стать лучше)
Такие простые задания последнее время не встречались. Это из 2020 года. Сейчас разметка будет в разы сложнее, зададут какой нибудь вычурный макет в фигме, Dugger Hilt обязательно, сохранять в бд обязательно, модули по фичам, и несколько экранов с различными recycler'ами. И тд. Плюс к этому всё чаще Compose. А, тесты не забудь. Поиск какой нибудь извращенный. Архитектура - чистая, как слеза, туда же.
Тестовое этого года. В большинстве случаев встречаются +- такие тестовые, но бывают и большие тестовые задания где нужно показать умения владеть всем что понадобится для разработки, думаю их полезно решать для своего опыта, чтобы потренироваться, но для трудоустройства, не думаю.
Вообще странно, когда дают большие тестовые без внятных дальнейших перспектив, но кому интересны пожелания человека который устраивается на позицию android developer без вменяемого опыта, сейчас такой рынок с такими условиями, не будешь решать большое тестовое, кто-нибудь другой решит.
Остается надеяться что количество сделанных тестовых перерастет в качество и в приглашение на работу)
не будешь решать большое тестовое, кто-нибудь другой решит.
Там сейчас килополя кандидатов на стажировку-Junior позиции. Требования примерно как раньше были к middle++. Работодатели могут "перебирать харчами", так как много (очень много) хороших кандидатов. Сейчас это больше выглядит, как нанять мидла за ЗП стажера, или вообще бесплатно для начала. Видел уже стажировки, где нужно самому кандидату платить за участие)))
Почему 2 раза implementation viewmodel-ktx?
Способ неплох, но он для совсем новичка в разработке. Тут есть и большие, и маленькие проблемы.
Начнём с того, что задание даёт выбор библиотек, но лучше уточнить конкретные у работодателя. Это будет небольшой плюсик в копилку.
Начну сразу с минусов - в проекте слишком много библиотек, которые никак не используются. Для проекта было достаточно appcomap+material+vm-ktx+retrofit. Корутины идут из коробки с котлином, остальные просто не нужны. Для работы bottomNavigationView необходима зависимость от материала.
Очень плохое распределение файлов. Гугл-курсы хорошие, но лучше посмотреть их сурсы :) Базово у тебя должно быть распределение хотя бы на data/domain/presentation, а дальше чисто смысловое распределение. Например MainFragment и иже с ним можно было засунуть в конкретную папку и дать ей название в соответствии со смыслом (например ScheduleFragment) и уже внутри этой папки сделать дополнительно папку для адаптера. Так у тебя ничего не потеряется и всё будет в логичных местах.
Затрону немного использование VMFactory и retrofit. Понятно, что отсутствует хотя бы репозиторий, но вот непонятный инжект через App не есть гуд. Можно было сделать адекватный инжектор и провайдить это всё в onCreate у фрагмента, а не засовывать в App. Так же вместо непонятной функции в App можно было сделать нормальный экстенш у фрагмента.
Есть ещё куча мелочей, по типу работы с потоками (точнее её нет), правильная обработка ответов и подобное. Ещё замечу, что используются дата-классы, но не используется их потенциал и в итоге в проекте компилируется куча бесполезного кода.
В целом, тут не помешало бы код-ревью и желательно не от работодателя, потому что по какое ТЗ, такой и результат в итоге :)
Большое спасибо за содержательный комментарий! Да, действительно, много недочетов есть, но это одно из моих первых тестовых.
Считаю, написать такой пост было очень полезно, поскольку я получил много хороших комментариев, это позволило мне увидеть недочеты которые я не замечал до этого)
Извините, а что неадекватного в инжекте через Арр?
И экстэншн никогда не будет лучше нормального метода, описанного в классе, хотя бы потому, что это статика.
Извиняю :D В свою же очередь я прошу прощения за некропостинг :D
Но теперь давайте без шуток. Если мы производим инжект в onCreate у наследника Application (а onCreate вызывается при холодном старте), то получим очень долгий старт приложения. Игра в onCreate - это сложный баланс скорости старта приложения, при любом долгом или ресурсоёмком инжекте старт приложения становится всё дольше и дольше. А это, в свою очередь, влияет на пользовательский опыт.
А ещё о VM определённого экрана должен знать только этот экран. Естественно, если это не sharedVM, но обычно такое не нужно.
Хорошо, у нас есть метод в классе. Если этот метод используется внутри класса - круто, это нам нравится. А если этот метод понадобился в другом месте? Хмм, хорошо. Мы сделали его публичным и... Получили инит класса в переменную, хранение её в памяти ради одного метода. А если она нужна сразу в 50 местах? Непорядок. И вот мы пришли к статике, которая не требует создания 50 новых переменных ради одного метода.
А теперь в суровую реальность: этот метод провайдит VM, хорошо. Но почему в App? И зачем вообще делать такую функцию, когда она никак не упрощает код (точно так же можно вызвать VMProvider и внутри Activity/Fragment) и просто занимает место..? Ладно, допустим она нам нужна. А как мы получим конкретный экземпляр этого класса? Через статику. Хотя нет, можно сделать так:
(activity?.application as App)И получить nullPointerException. Ну да бог с ним, представим, что инстанс application&activity всегда существует. Да, мы получили нашу функцию. А она правда наша? А если это уже не изначальный App, а пересозданный? А не получим ли мы в этом моменте ошибку?
private lateinit var viewModelFactory: MainViewModelFactory
private lateinit var retrofit: ApiServiceИ как нам быть тогда? Правильно. Статика и ещё раз статика. Но даже этого всего можно было избежать. Как? Правильно, выкидываем App к чертям (он никак не используется) и используем VMProvider внутри фрагмента, а ретрофит создаём при первом обращении и в дальнейшем используем его инстанс.
Прямо то что в глаза бросается.
Не нуллабельные поля в дата классах(которые ответ сервака)
Апп тут не нужен
findViewById в адаптере
За нейминг mBinding четыре минуса)
Чё то ещё было, но лень опять смотреть. В целом код выглядит на junior+
Разъясните пункт 1. Делая все поля нуллабельными мы усложняем себе жизнь, делая мыслительную работу за статический анализатор. Если известно, что какие-то поля точно будут заполнены, почему бы не облегчить себе жизнь и убрать нуллабельность?
потому что мы не отвечаем за бэк. И в поле может прилететь нулл. И при использовании gson, даже если задать значение поля по умолчанию это не гарантирует того что в ненуллабельном поле не появится нулл) да и работа с нуллабельностью не на столько сложная, чтоб ее избегать, а вот защитить от npe в будущем это может очень сильно
Мне кажется, что по какой-то причине бэкенд может не отправить или вернуть значение null, и в таком случае приложение не должно крашнуться, как минимум. ИМХО.
"Апп тут не нужен"
Откуда в таком случае зависимости получать?
Мне, как начинающему специалисту, очень зашло. Пока в начале пути, не всегда есть понимание как подступиться к решению.
Было бы здорово увидеть разборы и других заданий.
Разбор тестового задания на позицию Android Developer