Пробное Android-приложение на Kotlin

Пробный пример андроид приложения на Котлине

Никита

Здесь я вижу очень много граблей; на некоторых из них я танцевал больше года.

Безусловное создание фрагмента в Activity.onCreate

override fun onCreate(savedInstanceState: Bundle?) {
    super.onCreate(savedInstanceState)     // (1)
    setContentView(R.layout.activity_main)

    fragmentManager
            .beginTransaction()
            .replace(R.id.fragment, LoginFragment())
            .commit()                      // (2)
}

При первом старте Activity всё в порядке: в месте (2) создаётся и добавляется фрагмент. При рестартах после смены конфигурации (поворота экрана) фреймворк сам восстановит фрагмент в месте (1), после чего восстановленный фрагмент (там мог быть уже другой фрагмент, а за ним — сформированный действиями пользователя backstack) будет заменён новым в месте (2). Возникнет иллюзия, что состояние фрагмента не сохранилось. Нужно создавать фрагмент только при первом старте:

if (savedInstanceState == null) {
    fragmentManager
            .beginTransaction()
            .replace(R.id.fragment, LoginFragment())
            .commit()
}

Передача данных через singleton

val singleton = ProductSingleton
singleton.id = product.id
singleton.img = product.img
singleton.text = product.text
singleton.title = product.title
val intent = Intent(context, DetailProductActivity::class.java)
context.startActivity(intent)

Минусов у такого подхода полно.

• При поддержке кода (добавлении полей или мест запуска DetailProductActivity) можно забыть присвоить один из компонентов (id, img, text, title). Тогда ему достанется пустое или оставленное предыдущим разом значение.
• Такую Activity нельзя открыть из другого приложения.
• Такую Activity нельзя открыть несколько раз (положим, у Product есть рекомендуемые товары, которые снова ведут на DetailProductActivity, создавая стэк из нескольких Activity).
• Самое фатальное. Когда Android решит убить, а затем оживить процесс приложения, синглтон окажется пуст. Как раз вчера обсуждали вредный совет использовать статику.

Правильное решение — реализовать у передаваемого объекта интерфейс Parcelable (благо, в экспериментальном режиме работает @Parcelize) и передавать через extras.

Советы по работе с SharedPreferences

getDefaultSharedPreferences(context) отдаст SharedPreferences всего приложения. А, например, Activity.getPreferences(int) — префы для текущей Activity. Чтобы вообще не касаться API Android в этом шатком месте, я использую Kotpref.

Неотписка

Презентеры запускают асинхронщину (лезут в интернет), но не отменяют её, когда view уничтожается. Если запустить асинхронную операцию и уйти (повернуть экран, нажать Home или Back), можно получить state loss. Все асинхронные операции нужно отменять в onDestroyView.

Неправильное использование null-safety

• Зачем разрешать нулевой лист в fun initAdapter(products: List<ProductReviewModel>?), какой толк от данных без данных?
• Зачем разрешать нулевой ViewHolder?

override fun onBindViewHolder(holder: MyAdapter?, position: Int) {
    holder?.bindImage(products[position].img)
    holder?.bindText(products[position].text, products[position].text)
    holder?.bindOnClick(products[position], context)
}

Ой! Почему класс ViewHolder'а называется MyAdapter?

• Какой смысл от нулевого itemView, особенно если потом стоят non-null assertions (!!)?
• Ну и ещё. И вот тут.
• Все эти параметры должны быть non-null. И это точно.

Именование

• val singleton = ProductSingleton. Кроме того, что Singleton — плохой шаблон проектирования, singleton — это ужасное имя для свойства. Бессмысленно, как someObject или thing. Кстати, почему свойство не приватное?
• mDetailProductPresenter, mLoginPresenter, mProdListPresenter. Джейк не рекомендует использовать такого рода венгерскую нотацию, а Седрик не рад, что привнёс её в Android. Кроме того, она использована неправильно: m положено ставить перед именами непубличных полей, а здесь свойство публично (по ошибке?)
• Нет смысла называть классы словом Model. Они должны называться так, чем являются: UserCredentials, ProductInfo, ProductReview и т. п.. И зачем везде модификатор data? Используются ли по факту hashCode, equals, toString, copy, destructuring?

Хардкод

• Адрес сервиса захардкожен. Лучше выносить такого рода константы в buildConfigField.
• Тексты надо выносить в ресурсы. А слова retings не существует.

Странности

• fragment.arguments = args. Зачем ставить пустые аргументы? Если там раньше были аргументы, а теперь нет, то их просто не нужно ставить. Если они там будут потом, то и Bundle нужно будет создавать потом. Это звучит довольно очевидно, но есть люди, которые дрожат над каждой строкой своего кода и никогда его не удаляют. Это неверно, в процессе эколюции проекта ненужный код должен умирать. Написать пару строк — не проблема.
• Зачем явно использовать findViewById, когда подключены Kotlin Android Extensions?
• Один-два класса в пакете. Зачем им такая изолированность?

Вёрстка

• Пару LinearLayout + RelativeLayout можно легко заменить одним FrameLayout.
• ConstraintLayout здесь бесполезен.

Прочее

• Всё за один коммит. Ну, как это обычно бывает у программистов.
• Кажется, /src/main/AndroidManifest.xml не используется.
• Экземпляры Retrofit стоит кэшировать, а не создавать каждый раз новые.
• minifyEnabled false. Дичайше рекомендую подружиться с ProGuard. Маленькие приложеньки, использующие AppCompat, можно ужать до 300-500 КиБ. (Если это кому-то кроме меня интересно.)
• В libs лежит неиспользуемая библиотека.