Best Practices

Введение

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

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

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

warning

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

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

Общее

Следите за неймингом
У вас не должно быть переменных и методов, которые не несут в названии никакой информации о том, для чего они предназначены.
Например: binding.button.setOnCLickListener { buttonClickAction() } - ни кнопка, ни метод не несут абсолютно никакой информации о том, к чему они относятся
Кликабельные элементы UI не должны сами решать, какой метод вьюмодели им вызывать
- exitButton.setOnClickListener { viewModel.clearUserData() } - UI не должен говорить вьюмодели - чисти данные пользователя
- exitButton.setOnClickListener { viewModel.onExitButtonPressed() } - UI должен говорить вьюмодели - нажата кнопка exitButton
- Публичное API вьюмодели должно быть таким, чтобы по нему явно было понятно, когда его использовать
Максимально настраивайте UI элемент в .xml или .xib, чтобы не заниматься его настройкой в коде
Используйте аннотации @Throws только в случае, если это попадает в Swift код
Используйте weakLambda, чтобы сохранять receiver слабой ссылкой
Не создавайте общий класс констант - рано или поздно получится свалка.
Константы должны создаваться именно там, где они имеют смысл
Продумывайте имена функций и их содержимое
- Памятка
- Функция должна выполнять какую-то одну конкретную задачу, не должно быть функций, которые делают по 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
- somethingButton.setOnClickListener { somethingButtonPressed() } - устанавливайте действие на клик одной функцией, не пишите логику сразу в setOnClickListener
Не используйте форскасты
О том, когда их действительно нужно использовать вы поймете, когда дорастёте до мидла :)
Используйте KeyValueStorage правильно
Данные из KeyValueStorage не должны сохраняться в свойства класса, каждый раз надо обращаться к KeyValueStorage и получать актуальное значение.
Создавайте кастомные Exceptions правильно
Кастомные Exceptions, которые вы сами кидаете, наследуйте от RuntimeException(), либо от более конкретных его наследников.
Также, не забывайте прокидывать в родителя cause реально произошедшей ошибки, чтобы потом увидеть ее в stackTrace
Не забывайте про конфликты имен Kotlin и iOS
- Если у вас в common коде будет свойство description, вот как оно будет доступно на iOS
- Одинаковые имена переменных в двух разных интерфейсах
- Функции с одинаковыми именами аргументов, но разными типами
- Классы с одинаковым именем, но в разных пакетах
Связь UI с viewModel должна полностью находиться в функции bindToViewModel
Функция bindToViewModel должна быть легко читаема, поэтому должна вызывать другие функции, которые связывают конкретный UI элемент с viewModel
Константы должны иметь такое название, чтобы по нему было понятно, для чего они нужны
- Плохое название константы: 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
- Если версии подов не будут явно обозначены - то у нового разработчика, или у тебя (на другом компе) при установке подов могут подтянуться более новые версии. Есть вероятность, что в этих новых подах что-то будет изменено и проект не скомпилируется. Либо, как сейчас популярно, в библиотеку всунут зловредный код в новой версии он тоже скачается :)
Называйте ViewController-ы правильно
Все ViewController-ы должны называться с окончанием ViewController, а не Screen, потому что Screen- это экран девайса. ViewController не обязательно занимает весь экран, их на экране сразу несколько: твой собственный, UINavigationController, UITabBarController, модалки
Называйте @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

Во Fragment-ах аккуратно выбирайте скоуп запуска корутины - если нужно работать только пока есть view - используйте viewLifecycleScope, а не lifecycleScope.
Обрабатывайте стейты на Android правильно
Подписка вью на вьюмодель должна создаваться сразу, как только мы их создали, в методе onViewCreated.
Тема приложения должна выставляться в AndroidManifest, а не в Activity и Fragment-ах.
На чистом Android при работе со списками не забывайте смотреть сюда
Константы класса должны находиться в сompanion object, а в свойствах должно быть все то, что уникально для каждого экземпляра класса

Используйте конструкцию `with(binding...)` для `XML` элементов правильно. Не стоит ей злоупотреблять, иначе она будет только ухудшать читаемость

не стоит использовать

private fun setToolBar() {
    with(binding.toolbar) {
        navigationIcon = AppCompatResources.getDrawable(
            requireContext(),
            R.drawable.arrow_back
        )
        setNavigationOnClickListener {
            findNavController().navigateUp()
        }
        setOnMenuItemClickListener { menuItem ->
            when (menuItem.itemId) {
                R.id.action_logout -> {
                    viewModel.onLogoutPressed()
                    true
                }
                else -> {
                    false
                }
            }
        }
    }
}

стоит использовать

with(binding.view1.subview1.subsubview1) {
    label1.text = TODO()
    button1.setOnClickListener { TODO() }
    image1.imageAlpha = TODO()
    view11.subview1.button1.setOnClickListener { TODO() }
    view11.subview2.button2.setOnClickListener { TODO() }
}

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

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

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

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

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

Если в вашем приложении есть логика по выбору стартовой навигации
- Не забывайте использовать savedInstanceState, чтобы не создавать граф навигации заново
- Убедитесь, что установили граф в activity_main.xml
Если вы не используете navigation-safe-args-gradle-plugin
- Аргументы фрагмента получайте через requireArguments()
- Если какого-то аргумента нет - кидайте кастомную ошибку, что нет конкретного аргумента
- Используйте вычисляемые свойства для работы с аргументами
  private val something: String get() = requireArguments().getString(SOMETHING_KEY) ?: throw NoArgumentsException(lostArgument: SOMETHING_KEY)
Используйте lateinit правильно
lateinit - это чисто андроидная штука, костыль, чтобы передавать компоненты во фрагменты и активити. Нужна потому что мы не можем создать кастомный класс фрагмента или активити, чтобы передавать зависимости сразу в конструктор. Мы даже не можем точно предсказать место, где будет создан объект фрагмента/активити. Поэтому lateinit - это костыль, которого нужно избегать, потому что lateinit переменную можно забыть проинитить и приложение крашнется
Настраивайте Toolbar правильно
Чтобы полностью взять на себя настройку Toolbar - укажите тему NoActionBar и, вместо ActionBar-а системного, используйте Toolbar - UI элемент который полностью тобой управляется. Также, у него есть встроенная интеграция с Navigation Component

UI

xml называйте аналогично названию класса
- Плохой нейминг: DetailRepoInfoFragment и fragment_detail_repo
- Хороший нейминг: DetailInfoFragment и detail_info_fragment
Отступы всегда должны быть кратны 4 (8, 16, 24, 32), если в дизайне по-другому, задавайте вопросы
Если какой-то набор элементов используется на нескольких экранах - выносите его в отдельный .xml и подключайте с помощью include
Используйте константы для отступов в приложении правильно:
- Если дизайнер обозначил, что есть общие значения для некоторых отступов и т.д. - используйте константы
- Если обозначения общих размеров и отступов нет - использование констант на свой страх и риск (не рекомендуется)
- Констант не должно быть слишком много, иначе они будут только путать
- Именоваться константы должны относительно контекста использования - background_color, status_bar_color, button_color_default, default_top_margin и т.д.
Используйте tools
Во всех UI элементах, которые содержат поле text, устанавливайте текст используя tools:text, чтобы было легче ориентироваться в дизайне экрана
Избегайте вложенности при использовании ConstraintLayout
Главная цель ConstraintLayout в том, чтобы не использовать вложение в LinearLayout для расположения элементов на экране. Нужно это, во-первых, для улучшения производительности, потому что при большой вложенности layout друг в друга она сильно падает. Во-вторых, для более удобной верстки и улучшения читаемости xml
Верстайте экран сверху вниз, констрейнты вьюхам устанавливайте относительно друг друга, а не привязывайте каждую к корневому Layout
- Если элемент всегда расположен внизу экрана - не надо цеплять его к верхнему элементу
- Чтобы убедиться, что ваши правила верстки правильные - проверяйте на экранах разного размера - большой, средний, маленький, для этого просто переключайте превью
Используя свои стили, всегда наследуйтесь от дефолтного, чтобы не потерять его настройки
Не указывайте размеры width и height в стилях
Устанавливайте размеры вьюх правильно
- Размеры вьюх на Android не хардкодятся, как на iOS
- Фиксированные размеры устанавливаются только у картинок
- У всех остальных элементов высота - wrap_content, ширина - 0dp - чтобы элемент растягивался по констрейнтам

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

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

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

Best Practices

Введение​

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

warning

Общее​

iOS​

Logic​

UI​

Android​

Logic​

UI​

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

Введение

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

Общее

iOS

Logic

UI

Android

Logic

UI

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