Best Practices

Введение

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

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

Критичные пункты для ревью

warning

При нарушении любого из пунктов данного списка ревью будет прервано и отдано на исправление без полной проверки оставшегося кода

Соответствие кодстайлу
Если приложение поддерживает не только портретную ориентацию, проверяйте все экраны на соответствие дизайну перед отправкой на ревью
Не должно быть никакого хардкода текста, который показывается пользователю, надо использовать строки локализации
Проверяйте, что не добавили в git ничего лишнего, настройте .gitignore
Код не должен содержать никаких дебажных print и закомментированных строк, которые не несут никакой смысловой нагрузки
(iOS) Все @IBOutlet должны быть приватными
(iOS) Extensions для каждого класса должны находиться строго в отдельных файлах, с соответствующим названием файла — ClassName+Extensions

Общее

Следите за неймингом
У вас не должно быть переменных и методов, которые не несут в названии никакой информации о том, для чего они предназначены.
Например: binding.button.setOnCLickListener { buttonClickAction() } — ни кнопка, ни метод не несут абсолютно никакой информации о том, к чему они относятся
Кликабельные элементы UI не должны сами решать, какой метод вьюмодели им вызывать
- exitButton.setOnClickListener { viewModel.clearUserData() } — UI не должен говорить вьюмодели — чисти данные пользователя
- exitButton.setOnClickListener { viewModel.onExitButtonPressed() } — UI должен говорить вьюмодели — нажата кнопка exitButton
- Публичное API вьюмодели должно быть таким, чтобы по нему явно было понятно, когда его использовать
Используйте аннотации @Throws только в случае, если это попадает в Swift код
Не создавайте общий класс констант — рано или поздно получится свалка.
Константы должны создаваться именно там, где они имеют смысл
Продумывайте имена функций и их содержимое
- Памятка
- Функция должна выполнять какую-то одну конкретную задачу, не должно быть функций, которые делают по 10 дел сразу и называются doSomething1AndDoSomething2AndDoSomething3
Не реализуйте то, что уже есть в системе
Если что-то в дизайне немного отличается от системного варианта Alert, Toolbar, кнопка "назад" — спросите у руководителя, действительно ли должно быть 1 в 1 как в дизайне.
В большинстве случаев системная реализация подходит.
Не работайте с сетевыми сущностями в приложении
- Преобразуйте сетевые сущности в свои — доменные и используйте их во всем приложении
- Если в приложении вы работаете с сетевыми сущностями, то в случае, если на сервере что-то изменят, например изменят имя какого-то поля или поменяют вложенность полей, то вам придется исправлять все места, где используется эта сущность.
  А если при получении сетевой сущности вы сразу преобразуете(маппите) ее в доменную, то при изменении сетевой вам нужно будет просто изменить функцию-маппер.
- Если от сервера где-то приходит null, вы можете как-то заменить его на not-null значение, чтобы удобнее работать в приложении. Например, если приходит поле description = null — в маппере можно заменить его на пустую строку
- Доменная сущность пишется независимо от серверной, она включает только то, что реально будет использоваться приложением, а не хранит абсолютно все данные, полученные от сервера
Используйте автоформатирование после изменения каждого файла
- Option + Command + L (Android Studio)
- Control + I + L (Xcode)
Нейминг переменных для State и Action
Если вы используете State и Actions, то либо просто называйте переменные вьюмодели state и actions, либо называйте так, чтобы в их имени обязательно фигурировало, что это state или actions.
Правила для заданий, начиная с 4ого блока
- Новые зависимости добавляйте сразу в libs.versions.toml в нужный блок
- Не используйте MR ресурсы на платформе, все ресурсы, которые нужны только на платформе получайте на Android через R, на iOS через R.swift
- В мультиплатформенных ресурсах должны быть только те, которые управляются общей логикой. напоминалка
Не допускайте сильной вложенности кода
- Используйте early return
- Избегайте callback hell
Не используйте форскасты
О том, когда их действительно нужно использовать вы поймете, когда дорастёте до мидла :)
Используйте KeyValueStorage правильно
Данные из KeyValueStorage не должны сохраняться в свойства класса, каждый раз надо обращаться к KeyValueStorage и получать актуальное значение.
Создавайте кастомные Exceptions правильно
Кастомные Exceptions, которые вы сами кидаете, наследуйте от RuntimeException(), либо от более конкретных его наследников.
Также, не забывайте прокидывать в родителя cause реально произошедшей ошибки, чтобы потом увидеть ее в stackTrace
Не забывайте про конфликты имен Kotlin и iOS
- Если у вас в common коде будет свойство description, вот как оно будет доступно на iOS
- Одинаковые имена переменных в двух разных интерфейсах
- Функции с одинаковыми именами аргументов, но разными типами
- Классы с одинаковым именем, но в разных пакетах
Константы должны иметь такое название, чтобы по нему было понятно, для чего они нужны
- Плохое название константы: const val REPO_NAME = "repo_name"
- Хорошее название константы: const val REPO_NAME_ARG_KEY = "repo_name"
Не надо выдавать никакие данные из элемента списка (например по клику)
По клику на элемент списка он должен просто информировать наружу — "на меня нажали"
Никогда не обрабатывайте Error-ы в catch
- У Throwable два наследника — Exception и Error. Exception нужно обрабатывать, они исправимы, а Error-ы — неисправимы, их обрабатывать не надо. Приложение должно упасть с информацией о том, что пошло не так (в Error эта информация есть)
- изучите документацию

iOS

Logic

Устанавливайте версии подов в Podfile
- Версии установленных подов можно посмотреть после установки подов в файле Podfile.lock
- Если версии подов не будут явно обозначены - то у нового разработчика или у тебя (на другом компе) при установке подов могут подтянуться более новые версии. Есть вероятность, что в этих новых подах что-то будет изменено и проект не скомпилируется. Либо, как сейчас популярно, в библиотеку всунут зловредный код в новой версии, и он тоже скачается :)
Называйте экраны приложения правильно
В UIKit-экранах используйте суффикс ViewController — это системная сущность, которая не обязательно занимает весь экран (их может быть несколько: ваш контроллер, UINavigationController, UITabBarController, модалки). В SwiftUI и на Android (Compose) для экранов приложения используйте суффикс Screen — это абстракция экрана приложения, а не экран девайса.
Называйте @IBOutlet-ы правильно
- Все аутлеты должны содержать в название тех данных приложения, которые будут в нем отображаться
- По названию каждого аутлета должно быть однозначно понятно, к какому классу View он относится
- Пример плохого нейминга для поля с описанием чего-либо — @IBOutlet private var label: UILabel!
- Хороший нейминг — @IBOutlet private var descriptionLabel: UITextView!
Обрабатывайте стейты на iOS правильно
SplashScreen делать в LaunchScreen.storyboard, а не в SplashViewController
Не устанавливайте ресурсы для UI элементов в xib (текст, картинки и тд). Также, все строки локализации должны устанавливаться во ViewController в отдельной функции localize()
Все изменения конфигурации проекта должны производиться в xcconfig
Не забывайте про циклы сильных ссылок
- Используйте weak self там, где он нужен
- Приведет к утечке памяти: cell.onTap = { self.navigationController?... }
- Не приведет к утечке памяти: cell.onTap = { [weak self] in self?.navigationController?... }
Убирайте текст кнопки "Назад" правильно
- Чтобы у кнопки "Назад" не было текста — нужно у предыдущего экрана выставить в navigationItem.backButtonTitle = ""
- navController берет название для кнопки "Назад" от того экрана, на который будет производиться переход назад
Ключи в классах всегда private static let
Ключи не являются уникальными для конкретного экземпляра класса, поэтому их нужно делать статическими
Работа с ресурсами из R.swift
- Когда вы добавили картинки, цвета и другие ресурсы в Assets, вы можете получить к ним доступ через R.swift, например: R.image.somethingImage()
- Некоторые ресурсы, такие как цвета, R.swift возвращает nullable. В этом случае guard обработку делать не нужно, потому что раз R.swift предоставил доступ к переменной, значит смог ее сгенерировать на основе цвета.
  Поэтому, чтобы узнать о том, что при наличии переменной ресурса самого ресурса нет — можно использовать форскаст. Таким образом, если все-таки произойдет ошибка — мы будем разбираться в причине, а если бы мы обработали null — то не узнали бы, что такая ошибка может произойти

UI

Проверяйте имя файла картинки, которую скачали с Figma
- Имя картинки должно явно обозначать, что это за картинка, аналогично с неймингом переменных
- Векторные картинки должны быть single scale
Используйте кастомные View
- Если какой-то одинаковый набор элементов используется сразу на нескольких экранах — выносите его в отдельную кастомную UIView и используйте на нужных экранах
- добавление кастомных вьюх должно быть не через код, а через Interface builder
В .xib добавляйте ресурсы только для отладки
- Добавляйте текст, элементы списка, картинки, чтобы легче ориентироваться на дизайне экрана. Не добавляйте такой текст, который должен быть в готовом приложении, чтобы потом не забыть поменять.
- Например, для названия кнопки ввода используйте не "Ввод", а "//Ввод", тогда, если бы вы забыли заменить этот текст, то сразу бы увидели это при запуске приложения
Если фон во всем приложении одинаковый — устанавливайте его правильно
- Вариант 1: установить в AppDelegate, navigationController?.navigationBar.barStyle = .black — сразу после создания контроллера устанавливаем цвет
- Вариант 2: в .plist установите UIViewControllerBasedStatusBarAppearance = NO и Status bar is initially hidden = NO, тогда вообще не придется устанавливать navigationBar.barStyle = .black

Android

Logic

В рамках Composable функций не используйте lifecycleScope для задач, связанных с UI. Для корутин, живущих только во время композиции, используйте LaunchedEffect или rememberCoroutineScope().
Обрабатывайте стейты на Android правильно
Подписка composable на ViewModel должна происходить прямо внутри composable, через state‑обёртки (collectAsState, observeAsState), а не вручную через launch/collect.
Тема приложения должна задаваться на уровне Activity (или в Application / Manifest), а не внутри отдельных composable.
Константы класса должны находиться в сompanion object, а в свойствах должно быть все то, что уникально для каждого экземпляра класса.

Не запутайтесь с удалением `Observer`

так ничего не удалится

private val request = MutableLiveData<...>()
request.observeForever {
    if (it != null) {
        ...
    }
}
request.removeObserver { }

а вот так удалится

private val request = MutableLiveData<...>()
private val requestObserver: Observer<...> = Observer { ... }
request.removeObserver(requestObserver)

Настраивайте Toolbar правильно
В Compose не используйте системный `ActionBar`. Верхняя панель должна реализовываться через `TopAppBar` внутри `Scaffold`, что даёт полный контроль и естественную интеграцию с Navigation Compose.

UI

Content функцию экрана называйте аналогично названию экрана
- Плохой нейминг: DetailRepoInfoScreen и RepoInfoContent
- Хороший нейминг: DetailInfoScreen и DetailInfoContent
Отступы всегда должны быть кратны 4 (4, 8, 16, 24, 32). Если в дизайне по-другому, задавайте вопросы
Если какой-то набор элементов используется на нескольких экранах — выносите его в отдельную Composable функцию и переиспользуйте
Не передавайте ViewModel глубоко в дерево функций. UI должен зависеть от данных, а не от ViewModel.
State hoisting — выносите состояние наверх
- Composable должны быть максимально «тупыми». Плохо:@Composable fun Counter() { var count by remember { mutableStateOf(0) } }
- Лучше:@Composable fun Counter(count: Int, onIncrement: () -> Unit)

Перед отправкой на ревью

Проверяйте, чтобы у MR не было конфликтов. Если конфликты есть, вам нужно смержить целевую ветку ("destination", главная/основная ветка) в ветку с вашими изменениями ("source", ваше ответвление от основной ветки) и исправить конфликты
При повторной отправке на ревью пройдитесь по всем предыдущим комментариям и убедитесь, что все исправили. Отвечайте на каждый комментарий, чтобы ничего не пропустить и чтобы ревьювер сразу понимал, что вы это исправили.
После каждого коммита выделяйте немного времени, чтобы отсмотреть все изменения на наличие ошибок, перечисленных на этой странице.
Обязательно обращайте внимание на критичные для ревью пункты. Ошибки в этих пунктах будут прерывать ревью.
После создания итогового merge request, отсмотрите все сделанные в нем изменения. Благодаря тому, что вы проверяли их после каждого коммита, это не займет у вас слишком много времени.
Чем чаще вы будете перепроверять себя, тем больше ошибок будете видеть и сможете не допускать их в будущем

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

Best Practices

Введение​

Критичные пункты для ревью​

warning

Общее​

iOS​

Logic​

UI​

Android​

Logic​

UI​

Перед отправкой на ревью​

Введение

Критичные пункты для ревью

Общее

iOS

Logic

UI

Android

Logic

UI

Перед отправкой на ревью