Kotlin: без Best Practices и жизнь не та. Часть 1

Привет, Хабр! Данная статья о наболевших проблемах при программировании на Kotlin. В частности, затрону несколько тем, вызывающих больше всего неоднозначности – использование it в лямбда-выражениях, злоупотребление функциями из файла Standard.kt и краткость написания vs. читаемость кода.

Предыстория


Я начал смотреть на Kotlin около года назад (начиная с Milestone 12) и активно применял его для написания своих Android-приложений. После двух лет написания Android-приложений на языке Java писать на Kotlin было глотком свежего воздуха — код был намного компактнее (никаких тебе анонимных классов, появились функциональные фичи), а сам язык намного выразительнее (extension-функции, лямбда-функции) и безопаснее (null safety).

Когда язык вышел в релиз, я без капли сомнения начал писать на нём свой новый проект на работе, попутно расхваливая его своим коллегам (в своей небольшой компании я единственный Android-разработчик, остальные разрабатывают на Java клиент-серверные приложения). Я понимал, что после меня новому члену команды придется учить этот язык, что на мой взгляд в данном случае не являлось проблемой — этот язык очень похож на Java и через 3-5 дней после прочтения официальной документации на нём уже можно начать уверено писать.

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

// Намного лучше читается, когда выход из функции следует сразу за единственным Safe-call ("?."), после чего идет получение имени отдельной строчкой
val user = response?.user ?: return
val name = user.name.toLowerCase()

// Хуже читается, когда сразу несколько разных действий совмещено на одной строчке
val name = response?.user?.name?.toLowerCase() ?: return

Так как я был единственным программистом, быстро понял эту закономерность и неявно выработал для себя правило предпочитать читаемость кода его краткости. Всё бы было ничего, пока мы не взяли на стажировку начинающего Android-программиста. Как я и ожидал, после прочтения официальной документации по языку он быстро освоил Kotlin, имея за плечами опыт программирования на Java, но потом стали происходить странные вещи: каждое code review вызывало между нами получасовые (а иногда и часовые) дискуссии на тему того, какие конструкции языка лучше использовать в тех или иных ситуациях. Иными словами, мы начали вырабатывать стиль программирования на Kotlin в нашей компании. Я считаю, что эти дискуссии возникали по той причине, что в документации, являющейся входной точкой в мир Kotlin, не приведено тех самых Best Practices, а именно когда лучше НЕ использовать данные фичи и что лучше использовать вместо этого. Именно поэтому я и решил написать данную статью.

Сразу хочу оговорить, что я не пытаюсь доказать истинность моих утверждений, а пытаюсь обсудить как же всё-такие правильно писать те или иные вещи на Kotlin.

Проблемы в языке


«It» сallback hell


Данная проблема заключается в том, что в Kotlin разрешено не именовать единственный параметр функции обратного вызова. Он по умолчанию будет иметь имя «it». Пример:

 /** Здесь параметр это callback, который принимает один параметр и ничего не возвращает */
fun execute(callback: (Any?) -> Unit) { 
    ...
    callback(parameter)
    ...
}

/** Пример вызова. Kotlin позволяет писать как execute { ... }, так и execute({ ... }), выберем более краткий вариант */
execute {
    if (it is String) { // Доступ к parameter через переменную it, проверка что он имеет тип String
        ....
    }
    ....
}

Однако когда мы имеем несколько вложенных функций, может возникнуть путаница:

execute {
    execute {
        execute {
            if (it is String) { // it относится к последнему по вложенности вызову execute
                ....
            }
            ....
        }
    }
}

execute {
    execute {
        execute { parameter ->
            if (it is String) { // здесь it относится уже к предпоследнему по вложенности вызову execute, так как параметр последнего имеет другое имя
                ....
            }
            ....
        }
    }
}

На небольших фрагментах когда это может не казаться такой проблемой, однако если над кодом работают несколько человек и такая функция с вложенным вызовом имеет 10-15 строчек, то легко потерять, кому же на самом деле принадлежит it на данном уровне вложенности. Ситуация ухудшается, если в каждом уровне вложенности используется имя it для какой-то операции. В этом случае понимание такого кода сильно ухудшается.

executeRequest { // здесь it - это экземпляр класса Response
    if (it.body() == null) return
    executeDB { // здесь it - это экземпляр класса DatabaseHelper
        it.update(user)
        executeInBackgroud { // здесь it - это экземпляр класса Thread
            if (it.wait()) ... 
            ....
        }
    }
}

Здесь приведена дискуссия на тему читаемости кода, использующего it. Мое мнение — it сильно помогает сокращать код и повышает его понятность для простых функций, но как только мы имеем дело со вложенной функцией обратного вызова, лучше давать имена параметрам обеих функций:

 // Простая функция
executeInBackgroud {
    if (it.wait()) ... 
     ....
}

// вложенная функция
executeRequest { response ->
    if (response.body() == null) return
    executeDB { dbHelper ->
        dbHelper.update(user)
        ...
    }
}

Злоупотребление функциями из файла Standard.kt


Для тех кто не знает, в файле Standard.kt находится множество полезных функций. Здесь приведено подробное описание для чего нужна каждая из них.

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

Первый пример — функция let, которая по сути выполняет 2 задачи: позволяет вызвать код, если какое-то значение не равно null и перекладывает это значение в переменную it:

response?.user?.let {
    val name = it.name // в it теперь лежит объект user
}

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

val user = response?.user ?: return
val name = user.name

В третьих, let добавляет лишний уровень отступа, что ухудшает читаемость кода. Почитать по поводу данной функции можно здесь, здесь и здесь. Моё мнение — данная функция вообще не нужна в языке, единственный плюс от нее — помощь с null safety. Однако даже этот плюс можно решить другими более изящными и понятными способами (предварительная проверка на null при помощи ?: или просто if).

Что касается остальных функций, то они должны применятся крайне редко и осторожно. Возьмем, к примеру, with. Она позволяет не указывать каждый раз объект, на котором нужно вызвать функцию:

with(dbHelper) {
    update(user)
    delete(comment)
}

// вышеприведенный код эквивалентен следующему:
dbHelper.update(user)
dbHelper.delete(comment)

Проблема начинается там, где данные вызовы перемешаны с другим кодом, не относящимся к объекту dbHelper:

with(dbHelper) {
    val user = query(user.id)
    user.name = name
    user.address = getAddress() // getAddress() не относится к объекту dbHelper
    ....
    update(user)
    val comment = getLatestComment() // getLatestComment() также не относится к объекту dbHelper
    ....
    delete(comment)
}

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

О других наболевших вещах напишу в следующей статье, потому что это уже успела разрастись.

Update:

Так получилось, что пока я подготавливал материал для второй части статьи, было опубликовано видео презентации Антона Кекса, которое не только полностью затрагивало все пункты моей второй статьи, но и содержало некоторые дополнительные важные моменты. Но самое главное, что в этом видео также есть комментарии разработчиков. Я решил, что вторая статья будет уже не в том формате, что первая (а будет говорить что есть такая-то проблема, в видео об этом сказано на такой-то минуте), так что пока что я не буду писать второй части, по крайней мере пока не обнаружу новых проблем в языке. Всем, кто ждал продолжения, советую просмотреть видео презентации.
Поделиться публикацией
Похожие публикации
Ой, у вас баннер убежал!

Ну. И что?
Реклама
Комментарии 30
  • +5
    Есть такая проблема.
    Если послушать доклады Бреслава, он обычно говорит, что они стремились по максимуму все делать явно всегда, если только это не должно происходить в любом случае (например, автокасты).
    В защиту можно сказать, что это далеко не новая проблема и в Котлине она стоит далеко не так остро, как в груви.
    Ну и мои пять копеек: даже с именованными переменными стоит избегать многократно вложенных функций, оно плохо читается от природы.
    • +1
      По поводу описанных в статье проблем — с согласен с Вами, что их можно отнести к проблемам средней (или даже минимальной) важности. И всё-таки я считаю, что инструмент, которым ты пользуешься, должен содержать как можно меньше подобных проблем. К сожалению, в приведенных мной ссылках так или иначе написано, что в языке ничего не будет изменено для соблюдения обратной совместимости, поэтому единственное, что остается — осветить эти проблемы для программистов, чтобы они по возможности старались избегать их.

      По поводу вложенности — полностью с Вами согласен.
      • 0
        Многократно вложенные функции помогают в билдерах. Яркий тому пример — Anko
      • +7

        Самая дичь, с которой приходится жить — это возможность переопределить get() у val и возвращать значения динамически, что нехило ломает концепцию неизменяемых значений и иммьютабилити в принципе :(


        val a: Int
          get() = Random().nextInt()

        Особенно печально, когда val определен в интерфейсе и ты не знаешь, пока не заглянешь в реализацию, immutable этот val или нет.

        • 0
          Это явная стрельба в ногу, в лучшем случае о таком val должно быть написано в комментариях.
          Кстати, в бетах много методов в Java-классах были заменены на val, но потом их стало меньше.
          • +1
            Если я правильно понял что вы пытаетесь сделать, то с котлином всё ок — это вы пытаетесь сломать концепцию неизменяемых значений.
            val не должен меняться => 2 доступа к одному val должны возвращать одно и то же значение.
            • 0
              Тогда со всем чем угодно ок — просто некоторые пытаются сломать концепцию использования.
              • +1

                Ээ, я ничего не ломаю, язык позволяет так делать, а нам приходится с этим мириться.

                • +1
                  Не то чтобы мы от этого страдаем. Мне кажется предположение о неизменяемости само по себе неправомерно во многих случаях, например `List.size`.
                  • +1

                    Было бы гораздо приятнее иметь что-то вроде readonly var для таких случаев, а val оставить полностью иммьютабл

                  • –1
                    О боги!
                    Я не мог правильно прочитать сообщение, на которое ответил — я просто представить себе не мог, что в каком-то языке так можно делать.
                    Это же ужас!
                    Спасибо за пояснение.
                    Нет, я знал, что в котлине не уважают иммутабельность (те же их билдеры не могу существовать без мутабельности, что меня удивило при разборе HTML Builder), но что всё настолько запущено я не предполагал.
                • +2
                  В чистой Java какой-либо геттер не обязан каждый раз возвращать одно и то же, даже если у него нет сеттера. (Если только это явно не сказано в документации). Так что неизменяемость для val только в том смысле, что нельзя что-то изменить снаружи операцией присваивания этому полю.
                  • +1

                    И?


                    В Kotlin val часто работает как final поле в Java, вот final в таком случае оказывается более строгим, чем val, это и печалит.

                    • +1
                      val/var работают не как поля, а как свойства (peroperty) в C#, полей как части интерфейса класса в Kotlin вообще нет.
                      • +1

                        И? Я это понимаю :)


                        Бесит то, что Котлин весь такой модный и форсит иммутабельность (что классно), но понять, что объект реально immutable невозможно без чтения исходников/байткода. В Java это очень просто — public final поле это гарантирует (unless reflection). Подсказка со стороны IDE была бы очень не лишней (пойду тикет заведу).

              • 0

                Ну и зачем было делать этот it? Добавили проблему, теперь пытаются решить.

                • +3
                  Не совсем согласен, что it бесполезен и является проблемой. Для простых лямбда-функций она позволяет компактно записать ваше намерение. На официальном форуме приведен очень показательный пример, с которым я полностью согласен:

                  // Намного лучше читается так, почти как английский текст
                  (1..100).filter{ it > 50 }.map{ it * 2 }
                  // Хуже читается, появляются доп. символы
                  (1..100).filter{ x -> x > 50 }.map{ x -> x * 2 }
                  
                  • +1

                    Здесь it является плейсхолдером, как в Скале, типа как _ * 2 возвращает анонимную функцию с одним параметром. Плохо, что во вложенных функциях остается возможность сослаться на не тот if.

                    • +1
                      Возможности сослаться на `it` уровнем выше нету, переменная полностью экранируется.
                • 0
                  Спасибо за статью, жду продолжения. Как раз планирую начать изучать Kotlin и такие статьи очень помогают.
                  • +1
                    Спасибо за статью, приятно почитать аргументированное и конструктивное мнение.

                    По поводу «проблем» которые не будут чиниться — то что затронуто в этой статье не больше и не меньше чем code-style. То бишь что тут чинить если это by-design заранее обдуманные случаи использования.

                    `it` в принципе не предназначен для callback-ов, в документации так и написанно — именуйте параметр кроме простейших случаев. Функция `let` действительно редко встречается в моей практике, но она незаменима для цепочек вызовов с функциями принимающими аргумент, например `mylist.let {LinkedList(it)}.filter {it > 0}`. В случае с `with` мне кажется очевидным что `getLatestComment()` не относится к `dbHelper`, но если это не так, то действительно лучше обойтись явными вызовами по старинке.

                    В общем, я согласен с автором что иногда неприятности встречаются в коде, но не согласен что это вина языка. Надеюсь давно обещанный продвинутый форматер решит подобные проблемы.
                    • 0
                      Для случая с with есть некоторое решение, не слишком элегантное, но запутаться не позволит:
                      fun f() {
                          with ("string") str@ {
                              with (123) num@ {
                                  println(this@str.hashCode() + this@num.hashCode())
                              }
                          }
                      }
                      • +2
                        Согласитесь, что приведенный выше код добавляет просто именованные константы, просто объявленные экзотическим способом. Намного лучше читается код, где эти константы объявлены в привычном виде:

                        fun f() {
                            val str = "string"
                            val num = 123
                            println(str.hashCode() + num.hashCode())
                        }
                        


                        Также использование данного именования явно указывает на проблему в коде (тот самый момент, когда говорят, что код «пахнет»).

                        Спасибо за то, что напомнили, что можно ещё и так использовать with =)
                        • 0
                          Ну, можно написать Очень Умную Функцию того же вида, что и with, тогда данный рецепт поможет. Опять же, DSL.
                      • 0
                        Спасибо большое за статью, очень здравый, но редкий подход к оценке языка не по тому, как он позволяет писать в хороших случаях, а по тому, как плохо и непонятно он писать позволяет.

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

                        С удовольствием прочитаю следующую часть
                        • +1
                          Если вам понравился такой обзор языка, советую также почитать данную статью, которая как раз меня и вдохновила
                        • –1
                          Ох, а ведь казалось, что такой классный язык получиться должен был. Ведь разрабатывали очень крутые ребята. А вышло так себе. Взяли груви и немного приукрасили. Обидно.
                          • +1

                            Ну неееет. От груви он улетел гораздо дальше просто за счёт статичиской типизации

                          • +1
                            val user = response?.user ?: return
                            val name = user.name

                            как по мне но такие «ретурны» в коде могут быть очень малозаметными и в итоге код будет выполнятся иначе чем ожидает программист — да и найти такой «выход» в полотне кода (даже на 10 строк) будет сложно

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

                            Самое читаемое