Всем привет!

Я слишком ударился в архитектуру в последних постах, возвращаюсь с небес на землю)
Уже был пост как правильно писать unit тесты https://t.me/javaKotlinDevOps/33 и https://t.me/javaKotlinDevOps/43. Но кажется, что не хватает примеров. Начнем с антипаттернов - что не нужно делать в тестах?

Есть такая всем известная в Java мире библиотека для создания заглушек как Mockito. И у него есть среди всего прочего такая конструкция как

doNothing().when(mock).someMethod();

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

val mock = Mockito.mock(SomeClass.class);

как раз и является объект, который ничего не делает.
А точнее метод объекта не вызывается, а там, где нужно вернуть результат - возвращаются некие значения по умолчанию: https://site.mockito.org/javadoc/current/org/mockito/Mockito.html#2
А если так - явно писать doNothing() - это усложнять код теста, его читаемость и ценность как документации к коду. Держать в уме, что все методы заглушки, которые не переопределены в тесте, не делают ничего - проще и понятнее, чем явно перечислять их.

Теперь уже метод doNothing() выглядит бесполезным. Зачем же его тогда добавляли в библиотеку?
Есть два кейса:
1) когда вы используете spy, а не mock. Напомню, главное отличие spy - он по умолчанию вызывает методы существующего объекта.
Пример:

List list = new LinkedList();
List spy = spy(list);
doNothing().when(spy).clear();
spy.someMethodWithCLeaCall();

2) и об этой возможности я, признаюсь, не знал - в одном выражении можно задать несколько ответов на ряд последовательных вызовов одного метода заглушки:

doNothing()
.doThrow(new RuntimeException())
.when(mock).someVoidMethod();

//первый раз ничего не делаем
mock.someVoidMethod();

//бросаем RuntimeException во второй раз
mock.someVoidMethod();

В следующих постах будет продолжение...

#unittests #mockito #antipatterns

Всем привет!

Продолжим про антипаттерны в unit тестах.
Мы можем проверить в тесте метод на то, что он выбрасывает исключение.
В JUnit 5:

assertThrows(MyException.class, () -> myObject.myFunction(myString), "Expected myFunction() to throw, but it didn't");

и с помощью AssertJ:

assertThatExceptionOfType(MyException.class)
.isThrownBy(() -> { myObject.myFunction(myString); })
.withMessage("%s!", "boom")
.withMessageContaining("boom")
.withNoCause();

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

Но есть еще методы проверки, что исключения не было.
В JUnit 5:

assertDoesNotThrow(() -> myObject.myFunction(myString));

и AssertJ:

assertThatNoException()
.isThrownBy(() -> myObject.myFunction(myString));

А вот эти проверки выглядят излишними, т.к. выбрасывание исключения в любом случае прерывает тест, и он подсвечивается красным.
Возможно, если тест сложный и в нем много проверок - хочется локализовать место ошибки. Но в этом случае я бы задумался про разделение теста на несколько вместо использования assertDoesNotThrow. Опять же, в противном случае придется использовать данную проверку во всех тестах, кроме тех, что выбрасывают исключение. А это IMHO "замусоривает" код.

P.S. Интересный момент - библиотека Truth, о которой я писал ранее https://t.me/javaKotlinDevOps/51, вообще не содержит методов проверки на выбрасывание исключения, предлагая пользоваться соответствующим методом из JUnit, ведь assertThrows, описанный ранее, возвращает исключение:

val exception = assertThrows(MyException.class, () -> myObject.myFunction());

А уже его можно проверить более детально:

assertThat(exception)
.hasCauseThat()
.isInstanceOf(NumberFormatException.class);
assertThat(exception)
.hasMessageThat()
.startsWith("Bad");

Мне этот подход нравится.

#unittests #junit #assertj #truth #antipatterns

Всем привет!

Еще один антипаттерн в unit тестах - чрезмерное увлечение verify.

List<String> mockedList = mock(MyList.class);
mockedList.size();
verify(mockedList, times(1)).size();

Да, штука полезная. Полезная по большому счету в 2 случаях:
1) проверка тестируемого метода в случае, когда он ничего не возвращает. К слову - еще одна альтернативная проверка: для mutable объекта и меняющего состояние метода проверить поля SUT (System under test).
2) проверка важных шагов алгоритма.
Если с void методом все понятно, то с деталями алгоритма главное - найти ту черту, которая отделяет суть алгоритма от деталей реализации. И проверять только первое. Т.к. в противном случае мы получим "хрупкий" тест, падающий при каждом изменении реализации. Собственно, от этого критерия можно и отталкиваться. Пример: если назначение данного сервисного класса вызов удаленной системы, отбрасывание метрик и логов - то именно эти вызовы и нужно проверить. В других случаях вызов метода логирования и отправку метрики я бы не проверял.

Еще хороший критерий - проверять те вызовы, которые описаны в аналитике.

Ясно, что 100% гарантии того, что тест проживет в неизменном виде долго и счастливо это не даст, мы же не провидцы) Но частота исправлений теста снизится.

Есть еще одна радикальная альтернатива - вообще не тестировать в рамках unit тестов методы, где все, что мы можем проверить - это вызывался тот или иной метод. А оставить это на интеграционные тесты.

#unittests #antipatterns #mockito

Всем привет!

Еще одна проблема, которую я замечаю в unit тестах - это засилье copy-paste. Тесты есть, покрытие хорошее, тесты проходят. Все хорошо?
Не совсем.
Как я уже писал тест - это самая лучшая документация к коду. Поэтому если в тесте есть mock объект, скопированный с другого теста, но на самом деле не нужный для успешного прохождения теста - это вводит в заблуждение при изучении проекта. Аналогично - если есть лишние проверки verify, которые конкретно у данного теста всегда успешны или не проверяют ничего полезного, зато приводят к хрупкости теста.

Резюме: тесты - это тоже код, к их написанию нужно относится с той же серьезностью, как и к коду.

#unittests #antipatterns

Всем привет!

Есть такая отличная штука в Kotlin, как контекстные функции.
Вот документация https://kotlinlang.org/docs/scope-functions.html
Вот пример, хорошо иллюстрирующий зачем они нужны:

val man = Person("Vasya").apply {
age = 20 // same as this.age = 20
city = "Moscow"
}

Код стал проще, читается хорошо. Если, конечно, ты знаешь про контекстные функции)

Но как и любая вещь, контекстные функции могут быть использованы не только во благо(
Вот несколько антипаттернов:

val params = claim.systemState?.let {
FailureParams(
claim.partnerName,
it.name,
)
}

Что мне здесь не нравится - читаемость. Если читать сверху вниз, то получается, что мы берем статус из заявки и присваиваем переменной params ... не его, а совершенно другой объект, созданный внутри let. Скорость понимания кода страдает.

return claim.also {
saveToCache(it)
}

Опять же, мне не нравится читаемость кода. Мы возвращаем результат метода, claim. А нет, не возвращаем. Вначале пишем его в кэш. А потом уже возвращаем.
Гораздо проще было бы:

saveToCache(claim)
return claim

Ну и наконец самый хит:

return claim.also {
saveToCache(someOtherObject)
}

Зачем? Почему? Не понятно)

P.S. По ссылке выше есть неплохая табличка https://kotlinlang.org/docs/scope-functions.html#function-selection Это по сути навигатор по контекстным функциям - позволяет выбрать одну из 6 для вашего конкретного случая. На первое время точно будет полезной

#kotlin #antipatterns

Всем привет!

Сегодня пост о крутой фиче Kotlin, которая решила одну важную проблему. И добавила другую)
Я о Null safety.
Суть ее в том, что в Kotlin любой тип представлен в двух ипостасях - одна может содержать null значения, другая - нет.
String - не может содержать null,
String? - может.
Это два разных типа, неявное приведение второго к первому "без приседаний" невозможно.
Что дает Null safety?
По умолчанию предлагается использовать тип not null, и если так и делать, то кажется, что про NPE - NullPointerException - можно забыть. А заодно забыть о проверках на null как в коде, так и в тестах. Небольшой оффтоп - проверка not null значений на null в тестах - еще один антипаттерн. Говорит о том, что пишущий этот код еще не до конца познал Kotlin)
Вроде бы все хорошо. Но есть нюанс. Что будет, если присвоить not null переменной nullable значение? Ошибка компиляции. А всегда ли компилятор знает, что это nullable значение? Если тип Kotlin - то всегда. Если тип Java - то в общем случае не знает. Что же он делает в таком случае? А ничего, просто разрешает присваивание.

//Java
public class JavaNullability {
private Boolean value;

public Boolean getValue() {
return value;
}
}

// Kotlin
class KotlinNullability {
constructor(test: JavaNullability) {
val nullValue: Boolean? = test.value
val notNullValue: Boolean = test.value
// компилятор разрешает оба варианта
}
}

Что же будет в runtime? Привычный нам в Java NPE на втором присваивании. Да, null safety не защищает от NPE в Kotlin коде.

Что тут можно сделать?

1) при взаимодействии Kotlin-Java знать nullability Java параметров, с которым мы работаем. Это может быть описание контракта - OpenAPI спецификация или любой другой контракт.
2) явная проверка на null в коде присваивания
3) самый плохой вариант, антипаттерн - всегда присваивать значения из Java nullable типам в Kotlin. Почему это плохо? nullable типы расползаются по всей программе, и в итоге мы теряем все преимущества Kotlin null safety.

P.S. Но в целом: null safety - это круто! Подробнее можно о ней можно почитать в официальной документации: https://kotlinlang.org/docs/null-safety.html#nullable-receiver

#kotlin #java #antipatterns #nullsafety

Всем привет!

Хочу рассказать про наверное самый способ улучшить читаемость. Например, у вас есть сложное условие из нескольких уровней, каждый из которых состоит из ряда проверок. Или длинный метод с кучей условий, который сложно понять и на который справедливо ругается SonarQube.

Решение такое - выделяем атомарные блоки кода или условия в отдельные private методы.
К слову, данный способ является одним из базовых рефакторингов под названием "extract method" из известной книжки Фаулера "Рефакторинг".

Может возникнуть вопрос - а если метод вызывается один раз?
Ответ - без разницы.
У нас есть умный компилятор, он должен такие методы заинлайнить. Сразу скажу, спецификации, подтверждающей этот факт я предоставить не могу)

Окей, но даже если вам не повезет, и ваш компилятор этого не сделает - нужно иметь RPS (request per second) сильно больше 1, чтобы лишние вызовы метода сыграли свою роль. Причем даже в таком случае длинная цепочка вызовов методов должна быть в основном сценарии. Кажется, что под эти три условия попадает не так много кода. И в любом случае у нас есть НТ.

Еще может возникнуть вопрос с unit tests - их число тоже нужно увеличивать? Ответ - не обязательно, нет жесткой связки 1 к 1 теста и метода. Тест можно написать на группу взаимосвязанных методов, главное закрыть все возможные пути выполнения кода. Да и не нужно тестировать private методы.

И еще важный момент - extract method позволяет решить еще один часто встречающийся кейс. Предположим у вас есть дублирующийся код, отличающийся только одним небольшим блоком. Решение навскидку (на Kotlin):

fun doSomething(booleanParam: Boolean): SomeResult {
if (booleanParam) {
doAnotherThing()
}
// some code
}

Но приходится вводить Boolean параметр, а это хоть и не общепризнанный антипаттерн, по этому вопросу идут hollywars, но приближается к этому статусу)
Поэтому можно сделать так:

fun doSomething(): SomeResult {
// some code
}

fun doSomethingExtended(): SomeResult {
doAnotherThing()
return doSomething()
}

Итог: не нужно боятся выделять новые методы.

#refactoring #readability #antipatterns

Всем привет!

Я уже поднимал тему boolean параметров как антипаттерна https://t.me/javaKotlinDevOps/229. Давайте расширим ее до вопроса - когда стоит использовать if?
Является ли if антипаттерном?
По мнению некоторых товарищей - да, является: https://www.antiifprogramming.com/about-the-anti-if.php
Как по мне - не всегда, зависит от ситуации.
Чем плох if? // да, switch - это по сути тот же if.

1) может нарушать принцип Single Responsibility. Почему - думаю объяснять не нужно.
2) может ухудшать читаемость кода, я которую я всегда "топлю") Т.е. нарушает принцип KISS. Усугубляет ситуацию тот факт, что код как правило не остается неизменным. И обычный if else со временем может превратится в многоуровневого нечитаемого монстра.
3) может нарушать принцип Don't Repeat Yourself. Тут два очевидных варианта - либо во всех ветках if выражения есть дублирующийся код, либо чтобы обработать возврат некого метода всегда нужен if.
4) если в коде слишком много if (x != null) - это признак того, что вы неправильно работаете с nullability. Тут могу посоветовать Kotlin, т.к. он может сообщать о null значениях на этапе компиляции. Optional и его альтернативы в Java избавляют от NPE, но не избавляет от проверок на null. Я видел советы - просто не пишите код, который возвращает null - тогда проверки будут не нужны. Но это надежда на человеческий фактор, и компилятор (я про Kotlin) работает лучше)))

Да, я специально пишу везде слово "может". Бывают if-ы, которые не нарушают ни один из принципов.

Когда стоит волноваться?

1) подключаем SonarQube или Checkstyle и не игнорируем ошибки, связанные с цикломатической сложностью методов, см. https://t.me/javaKotlinDevOps/197
2) код просто сложно становится читать. Особенно хорошо эта проверка проходит на новых разработчиках)

Идеально конечно не писать код, приводящий к лишним if. Но я уже писал про человеческий фактор выше)

Что можно сделать? // будет некоторый повтор написанного тут https://t.me/javaKotlinDevOps/229

1) выделяем сложный код условия в отдельный метод.
2) вместо двух или более веток оператора if делаем несколько методов. Помогает в случае, если условно метод А всегда вызывает метод С с значением true, а метод Б - с значением false. Иначе будет как на знаменитой картинке - проблема не на моей стороне)))
3) используем not null объекты и переходим Kotlin
4) перепроектируем код, чтобы проверки выполнялись в одном месте, а не дублировались по коду. Для этого их придется перенести из вызывающего кода в вызываемый. И придумать правильное значение по умолчанию.
5) при необходимости вводим иерархию классов, чтобы каждый класс отвечал за одну ветку switch
6) используем паттерн Стратегия - по сути частный случай введения иерархии классов
7) используем паттерн Состояние (State), который кроме хранения состояния выполняет обработку, связанную с различными состояниями, тем самым убирая if из вызывающего кода

#antipatterns #if_antipattern #java #kotlin #solid #patterns #dev_compromises

Всем привет!

Нашел интересный антипаттерн, о котором ранее не задумывался. Coding By Exception - кодирование через исключения, не знаю как лучше перевести.
Суть в том, что под каждую ошибку заводится отдельное исключение. Почему это плохо?
1) В случае проверяемых исключений понятно - сигнатуры методов не "загрязняются", появляется искушение написать throws Exception.
2) Но ничуть не лучше ситуация в случае непроверяемых исключений. Для обработки исключений все равно приходится писать код. Чем больше исключений - чем больше количество такого кода. А если обработка единообразная для группы исключений возникает вопрос - а точно ли их надо разделять? Исключения хоть и должны быть свои, кастомные, но они должны охватывать какие-то типовые ошибки, а не каждую конкретную ошибку.
3) Если при любой ошибке бросать исключение - это может сказаться на производительности
4) Если при любой ошибке бросать исключение - это по сути goto программирование - переход в произвольное место кода. А goto - это "матерый" антипаттерн)))

#antipatterns