Кошелёк Mobile Challenge: итоги конкурса и подробный разбор решений командой разработки +10


У нас было две платформы, 1 000 000 рублей призового фонда, 6 призовых мест, 26 тысяч строк кода, которые нужно прочитать и оценить, а также 20 страниц фидбеков, несколько критериев оценки, ящик «Бадвайзера», пинта чистого эфира и 12 пузырьков успокоительного. Не то, чтобы всё это было категорически необходимо для проведения конкурса разработчиков, но если уж начал оценивать решения, то к делу надо подходить серьёзно.

Подводим итоги конкурса Кошелёк Mobile Challenge и в деталях разбираем решения участников.

Задание

Попробуйте передать ощущения физического кошелька в приложении «Кошелёк» и переосмыслить два экрана: главный экран со списком карт и экран с деталями карты.

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

Конкурс в цифрах

1 задание

2 платформы (iOS и Android)

1 000 000 рублей призового фонда

100 пользователей в телеграм-канале конкурса 

1 месяц на разработку решения 

13 участников

11 судей 

6 призовых мест

Победители

iOS 

1 место (250 000 рублей) — Антон Спивак 

2 место (150 000 рублей) — Роберт Шагинян 

3 место (100 000 рублей) — Павел Протасов

Android

1 место (250 000 рублей) — Андрей Эрдман 

2 место (150 000 рублей) — Виталий Кириллов 

3 место (100 000 рублей) — Георгий Ипполитов

Разбор решений

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

Напомним основные критерии оценки:

  1. Чистота и расширяемость кода.

  2. Плавность, скорость и отзывчивость интерфейса.

  3. Стабильность работы.

  4. Процент поддерживаемых ОС и устройств.

Технические требования:

  1. Нативное iOS- или Android-приложение, без кросс-платформы.

  2. Для Android: поддержка 23+ API.

  3. Для iOS: поддержка iOS 11+.

Разбор iOS решений

Соответствие техническим требованиям

Все представленные решения нативные, написаны на Swift, что не может не радовать. У большей части минимальная поддерживаемая версия ОС 11, что дает дополнительные баллы при оценке. А кто-то, вероятно, просто забыл указать, так как в проектах мы не нашли использования специфичного для новых версий API. Xcode же при создании нового проекта любезно ставит минимальной ту версию, которая для текущего SDK самая свежая.

Типичные ошибки и рекомендации

Несоблюдение принципа DI (Dependency Inversion) 

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

Нарушение принципа SR (Single Responsibility)

Это довольно холиварный принцип: не всегда очевидно, что считать единственной ответственностью, но встречались случаи, когда раздувание сущности дополнительной функциональностью затрудняет дальнейшее расширение. В таких случаях мы рекомендуем передекомпозировать/перепроектировать эту часть кода.

Создание лишних уровней абстракции

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

Рекомендуем не выделять протокол всегда, а только когда этого требует задача (задел на реализацию полиморфизма или сложность юнит-тестирования). В общем, мы за расширяемость (и уточнили это в критериях оценки), но не способом добавления протоколов к каждому классу и структуре в проекте.

Использование reference вместо value типов там, где в этом нет явной необходимости

Пример:

class RequestModel: Request {
    var method: HTTPMethod = .get
    var headers: [String : String]? = nil
    var url: URL = URL(string: "https://textures.cardsmobile.ru")!
    var parameters: [String : String]? = nil
    var contentType: ContentTypeRequestEnum = .applicationURLEncoder
}

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

Использование лишних 3rd party зависимостей

Многим разработчикам кажется, что библиотека Alamofire является стандартом индустрии для организации сетевого слоя. Она очевидно имеет ряд преимуществ и позволяет не изобретать велосипед каждый раз при работе с сетью в большом приложении. И все же, использовать Alamofire только для того, чтобы быстро написать AF.request(url).response { … } выглядит необоснованным решением. В чем отличие от URLSession.shared.dataTask(with: url) { … }.resume()? Кажется, что разницы никакой. При этом подключая такую большую библиотеку в своей проект, мы увеличиваем размер дистрибутива и потенциально затягиваем чужие баги. Хочется пошутить, перефразировав Билла Гейтса — «нативного URLSession должно хватить всем». Но как мы знаем, в любой шутке есть доля правды.

Неправильное сохранение контекстов в Core Data

Во время работы с несколькими контекстами (NSManagedObjectContext) при сохранении данных встречаются случаи последовательного вызова метода save() у контекстов. Оптимальнее подписываться на нотификацию NSManagedObjectContextDidSave и мержить данные с background контекста в главный view контекст. Опыт подсказывает, что сохранение через нотификации быстрее с точки зрения перформанса.

Рекомендуем для ознакомления книгу с описанием лучших практик использования Core Data.

Чрезмерное использование глобальных очередей DispatchQueue.global

В том числе из-за того, что очереди конкурентные, при частых вызовах метода async (загрузка картинки для каждой ячейки таблицы или коллекции), в какой-то момент работы программы можно обнаружить, что количество используемых тредов перевалило за несколько десятков, что снижает общую производительность приложения.

Не стесняйтесь создавать собственные очереди (желательно последовательные), чтобы избежать этой проблемы. За деталями рекомендуем обратиться к статье бывшего инженера Apple о советах по улучшению перформанса.

Использование background Quality Of Service (QoS)

Код в очереди с приоритетом background может при некоторых системных условиях не выполниться, что может повлечь за собой трудноуловимые баги (ведь кажется, редко бывает, что нам неважно, выполнится указанный код или нет). Лучше заменять на QoS utility. Подробности в треде.

Чрезмерная нагрузка главного потока задачами

Самые яркие примеры из предложенных решений — загрузка из сети и ресайзинг картинок на главном потоке.

Такие операции требовательны к ресурсам, отсутствие плохой сети на больших по размеру картинках может сыграть злую шутку и скролл будет подлагивать на списках. Рекомендуем разгружать главный поток, а вот — годная статья Image Resizing Techniques с сравнением различных техник ресайзинга изображений.

Игнорирование важных событий жизненного цикла приложения и UIViewController

Конкурсанты предложили логичную фичу — при открытии штрихкода увеличивать яркость экрана, чтобы он считывался легче. Для этого можно управлять яркостью в методах viewWillAppear и viewWillDisappear (для того, чтобы вернуть яркость в исходное состояние). Но, как показывает практика, обычно пользователь после предъявления карты просто сворачивает приложение. В таком случае яркость останется на максимуме, и одна звезда в App Store вам обеспечена. 

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

Отсутствие адаптивности под разные размеры экранов

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

Орфографические ошибки в коде

Частые ошибки затрудняют читабельность и ревью кода, снижает поиск нужных элементов по проекту. Понятно, что такие ошибки часто возникают из-за того, что «замылился глаз» или в состоянии потока быстро программируешь, пока не ускользнула мысль. Но всё же мы за код без ошибок (во всех их проявлениях), и поэтому рекомендуем включить проверку орфографии в Xcode с помощью Edit > Format > Spelling and Grammar > Check Spelling While Typing.

Проблемы с лейаутом констрейнов

В ситуациях, когда констрейны расставлены неверно, система не может однозначно их интерпретировать, чтобы понять финальный лейаут UI. Xcode помогает в этом и выбрасывает в консоль сообщения вида Unable to simultaneously satisfy constraints. Probably at least one of the constraints in the following list is one you don't want. 

Рекомендуем следить за этими сообщениями, а также ознакомиться со статьей Debugging Tricks and Tips.

Предупреждения (warnings) на этапе компиляции

Сюда могут относиться сообщения анализатора кода (swiftlint, если он у вас встроен в проект, встроенного анализатора об использовании deprecated методов и т.д.), обновление до рекомендуемых настроек проекта и т.д.

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

Неиспользуемый и закомментированный код в проекте

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

Магические константы

Как пример: выражение self.viewModel.objects().count%5 тяжело понять — почему именно эти значения используются, особенно когда возвращаешься к коду через какое-то длительное время. 

Как вариант, рекомендуем применить в этом месте метод рефакторинга «Замена магического числа символьной константой». Развернутое описание этого и других методов можно найти в книге «Рефакторинг. Улучшение существующего кода» Мартина Фаулера.

Что понравилось с технической стороны

Архитектура презентационного слоя

В большей части решений логика представления отделена от бизнес-логики, навигация между экранами тоже изолирована, что позволяет легко расширять количество экранов без изменения существующих. Многие выделяют такие составляющие, как View, Interactor, Model и Builder для сборки сцены и экрана.

Наличие билдера — плюс со стороны работы с DI (Dependency Injection). 

В одном из решений работа с зависимостями организована с использованием Dependency Container, что позволяет упростить работу с ними.

Сокрытие использования конкретной библиотеки за оберткой

В нескольких решениях использовалась библиотека Kingfisher для подгрузки картинок, только в одном использование было спрятано за классом ImageLoader. Это позволяет не размазывать 3rd party зависимость по всему проекту.

Использование ключевого слова final в декларации класса, метода или свойства

По стандарту при его отсутствии у класса разрешается создавать класс-наследник от указанного класса или переопределять метод в дочернем классе в рамках текущего модуля. Наличие ключевого слова final поможет компилятору оптимизировать код и выжать немного перформанса за счет использования статической диспатчеризации вместо динамической. При включении Whole Module Optimization в большинстве случаев компилятор справится сам и проставит final. Но в ситуациях, когда методы или свойства наследуемого класса делаются видимыми, но необходимо запретить их переопределение, то final будет полезен. Подробности в блоге.

Что понравилось с продуктовой стороны

Идеи организации списка карточек

#1 Размер карточки увеличивается при скролле, при двойном нажатии появляется штрихкод

#2 Горизонтальный скрол, дополнительно переходить на детали не требуется, чтобы показать штрихкод

Наличие офлайн-режима и быстрый запуск при холодном старте

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

Поддержка тёмной темы

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

Локализация

В некоторых решениях добавлена поддержка двух языков — русского и английского.

Поиск

Фильтрация

#1 Возможность показывать только просроченные или действительные элементы

#2 Фильтрация по категориям

Разбор Android-решений

Соответствие техническим требованиям

Все присланные решения нативные и написаны с использованием Kotlin, минимально поддерживаемый системный API 23+ использовали все.

Типичные ошибки и рекомендации

Почти все решения были сделаны без группировки по категориям. Данных апи было достаточно для организации удобного UI. Отсутствовала сортировка drag&drop.

Нарушение принципа DI (Dependency Inversion), повсеместное использование паттерна Singleton

В одном из проектов CardsInteractor из domain-слоя обращается напрямую в CardsRepository из data-слоя, что нарушает последний принцип SOLID. Для решения этой проблемы можно добавить Interface для CardsRepository, который будет находиться в Domain-слое.

Чрезмерное использование синглтонов ухудшает способность кода быть к тестированию. Дружеская рекомендация: прочесть книгу Роберта Мартина «Чистая архитектура» и изучить проект на github.

Монолитность проектов

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

Отсутствует разделение кода на слои Clean Architecture: data, domain, presentation

В одном из решений CardsInteractor обращается к Storage напрямую и загружает картинки из сети. Хотелось бы видеть тут разделение ответственности. Например, часть логики из CardsInteractor вынести в Repository и DataSource. CardsViewModelImpl отвечает за сортировку списка карт, что, вероятно, не должно выполнятся в презентационном слое.

Использование глобальных функций-расширений, таких как CharSequence?.isNotNullAndEmpty, List<T>.isEmpty 

На наш взгляд, использовать подобные функции —  избыточно.  Вместо них можно использовать, к примеру, функции-расширения языка Kotlin CharSequence?.isNullOrEmpty(), isEmpty() и т.д.

Использование неочевидных названий для переменных

Как пример view с названиями info1TitleTextView, info2TitleTextView, info3TitleTextView, info4TitleTextView. На первый взгляд тяжело сказать, чем отличаются данные view. Хочется видеть более говорящие названия.

Использование deprecated методов

Для примера в одном из проектов используется window.decorView.systemUiVisibility, View.SYSTEMUIFLAGLAYOUTFULLSCREEN, SYSTEMUIFLAGLAYOUTHIDE_NAVIGATION. Использование deprecated методов можно смело относить к техническому долгу, с которым рекомендуем бороться как можно быстрее.

Что понравилось с технической стороны

Многомодульность некоторых проектов и следование принципам Clean Architecture

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

Навигация

Во многих решениях сделаны плавные переходы между экранами в виде «общих» элементов.

Слава богам, что мы не нашли реализаций навигации через неявные интенты :)

Что понравилось с продуктовой стороны

#1 Хороший поиск и дизайн горизонтального скролла

#2 «3д» карта, интересный подход с быстрым доступом к штрихкоду из списка и общий элемент в навигации

#3 Понравилась группировка карт и удобный скролл к нужной группе


Спасибо за ревью команде жюри: Филиппу Шубину, Константину Степаненко, Константину Малкову, Николаю Ашанину, Андрею Бусику, Богдану Маншилину, Александру Пряничникову, Антону Давыдову, Александру Юдину, Семёну Задорожному, Кириллу Белоглазову. 

Спасибо всем участникам! Надеемся в будущем ещё порадовать сообщество разработчиков подобными активностями. 

А ещё недавно мы запустили телеграм-канал Cardsmobile | Кошелёк Engineering, в котором делимся опытом разработки, интересными историями и новостями не только команд iOS и Android, но также QA и backend. Рады там всем, нам будет интересно обменяться опытом решения подобных задач и с другими компаниями.




К сожалению, не доступен сервер mySQL