Kонструктивная и деструктивная критика в Code Review

Если простыми словами, то критика - это указание на недостатки (реже достоинства) чего-то, что сделал другой человек. Она бывает конструктивной и деструктивной.

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

Деструктивная критика, наоборот, бьет по самооценке. Она обвиняет человека и нападает на него. Деструктивная критика необъективна, не аргументирована и зачастую звучит унизительно.

Критику мы наблюдаем во всех сферах жизнедеятельности человека, особенно деструктивную в комментариях под моими видео 😁
Но где чаще всего мы ее встречаем в программировании? Правильно - это Code Review, где мы оцениваем код другого человека и оставляем свои комменатрии. И конечно же, нужно использовать только конструктивную критику во время Code Review.

Другими словами говоря, важно вести себя вежливо и уважительно, а также быть предельно ясным и полезным для разработчика, код которого ты просматриваешь. Один из способов сделать это - всегда комментируете ТОЛЬКО код и никогда не комментировать разработчика. Например:

❌Плохо:

Зачем ты использовал здесь многопоточность, когда очевидно, что от параллелизма нет никакой пользы?

✅Хорошо:

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

Хорошие комментарии помогают разработчику понять, почему ты их оставил (содержат объяснение), а также довольно часто направляют в сторону принятия более правильного решения (как в данном случае - использовать однопоточный код). А это как раз-таки и есть конструктивная критика 🙂

#dmdev_best_practices

Параллелизм - это сложно

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

Например, если бы мне нужно было решить проблему многопоточности, то я бы следовал следующим шагам:
1. Сначала глянул на существуюющий фреймворк, может ли он мне помочь с проблемой. Это самый безопасный вариант, который не только помогает избегать багов, но еще и предлагает кучу метрик из коробки.
2. Если нужно решить low-level проблему, то конечно же я открыл бы пакет java.util.concurrent
3. Если все еще недостаточно, то я прибегнул бы к synchronized блокам или volatile (для неблокирующей синхронизации)
4. Только здесь я бы начал думать о своем собственном решении, если ничего другого не помогло, предварительно глянув решения в других проектах (возможно, кто-то уже реализовал)

В любом случае, я уже советовал книгу "Java Concurrency in Practice" в своих курсах (Java Level 2), если кто-то хочет действительно разобраться в многопоточности. Ну и конечно же нужно очень хорошо понимать Java Memory Model.

#dmdev_best_practices

Избегай какой-либо логики в конструкторах

Самый простой и общепринятый вид конструкторов - это присваивание параметров полям объекта, которые имеют те же названия и типы.
Если ваш конструктор выглядит не так, то подумайте, возможно, он все-таки должен быть таким:

User(Long id, String username, LocalDate birthDate) {
    this.id = id;
    this.username = username;
    this.birthDate = birthDate;
}

Чем больше логики конструктор включает, тем сложнее тестировать такие классы. И особенно писать тесты на те классы, которые транзитивно зависят от них. Частично эту проблему решает Mockito, который позволяет создавать mock объект не вызывая конструктор. Но здесь есть несколько неприятных моментов:
- такой класс не может быть final
- лучше использовать настоящие объекты в тестах, вместо mock

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

Еще хуже выполнять в конструкторах IO операции, особенно сторонние вызовы в другие сервисы! Если это будет какой-нибудь Spring bean и произойдет исключение (например, сервис не отвечает), то контекст попросту не сможет подняться, что чревато в принципе неработающим приложением в продакшене. Такие ситуации я уже не раз встречал на своем опыте, и это действительно печально, когда даже откатиться на предыдущие версию не поможет восстановить работосопособность приложения.

Поэтому, вместо того, чтобы добавлять логику в конструктор, лучше перенести ее в какие-нибудь фабричные методы/классы, или вообще попросить у Dependency Injection framework нужные тебе параметры.

#dmdev_best_practices

Когда литералы выносить в константы

Литералы, такие как строки или числа (более подробно рассказывал про них в курсе Computer Science) можно либо просто оставлять в коде как есть, либо выносить их в константы и потом уже использовать именно константы.

В принципе, предпочтение нужно отдавать созданию констант, особенно если:
1. Одно и то же значение используется в нескольких местах, ибо это важно иметь one source of truth (иначе легко изменить значение в одном месте и забыть поменять в другом, из-за чего один и тот же функционал будет работать по-разному)
2. Значение требует пояснения, и гораздо лучше создать константы с хорошим названием и объяснением над ней, нежели оставлять неуклюжие комментарии прямо в месте использования литерала.

Например:

static final int GRACE_PERIOD_HOURS = 12;
scheduleSubscriptionRenewal(validUntil + GRACE_PERIOD_HOURS);

// или для сравнения:

scheduleSubscriptionRenewal(validUntil + 12);

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

Самые часто распространенные примеры, когда лучше оставлять литералы:
- true, false
- 0, 1
- SQL queries

#dmdev_best_practices

Операции должны быть идемпотентны

Идемпотентность означает, что выполнение одного и того же запроса несколько раз подряд приведет к тому же результату, что и выполнение этого запроса в первый раз. Сюда относятся любые операции и по любым протоколам, которые использует сервис для обработки внешних запросов. Например, REST API, gRPC, Thrift, Message Brokers (Kafka, Cloud Pub/Sub) и т.д.

Почему это так важно?

1️⃣ Идемпотентные запросы могут предотвратить случайное выполнение повторных операций, что может привести к ошибкам или нежелательным изменениям в системе. Обычно из приведенных протоколов общения - никто не гарантирует, даже Message Brokers, что одно и то же сообщение будет доставлено ровно один раз!

2️⃣ Идемпотентные запросы обеспечивают надежность операций, позволяя клиентам повторять запросы в случае неудачи или потери связи.

3️⃣ Идемпотентные запросы облегчают работу с API, так как клиенты могут повторять запросы без необходимости дополнительной логики для управления состоянием. Другими словами говоря, клиенту не стоит переживать и усложнять свою систему для отправки запросов сервису.

Если брать идемпотентность в контексте CRUD операций, про которые я довольно много рассказывал в курсе Spring:

- Read операции (HTTP GET) по умолчанию должны быть идемпотентными. Ибо никаких изменений на сервере не должно происходить при обработки запросов на чтение данных.

- Update операции (HTTP PUT) точно также легко сделать идемпотентными. Можно сколько угодно обновлять один и тот же объект с теми же самыми полями в запросе.

- Delete операции (HTTP DELETE) чуть сложнее, но удаление одного и того же ресурса не должно вызывать ошибки на сервере, если такого ресурса не оказалось во втором и последующих запросах

- Create операции (HTTP POST) сложнее всего добиться идемпотентности, поскольку их выполнение приводит к созданию нового ресурса с уникальным идентификатором. Повторный запрос на создание ресурса с теми же параметрами может привести к созданию дубликатов или ошибкам. Но все-таки способы есть, про которые я расскажу в отдельном посте!

#dmdev_best_practices

Обеспечение идемпотентности в Create операциях

Я думаю, что важность идемпотентности в Create операциях особенно высока в различных платежных системах, вроде Stripe, где повторные запросы на изменение баланса могут привести к таким плачевным ошибкам как дублирование платежей. Кстати, даже я на своем опыте встречался с этой проблемой - мне два раза перевели 10.000$ вместо одного (к сожалению, пришлось вернуть).

Почему так часто повторяются одни и те же операции?
- проблемы на стороне клиента, например, пользователю не понятно - была нажата кнопка "отправить" или нет, поэтому нажимает кнопку вновь
- запросы идут по сети, которая ненадежна
- соединение может оборваться в момент отправки сообщения, тогда клиент получит ошибку и будет пробовать переотправить сообщение вновь (retry mechanism)
- запрос может успеть дойти и обработаться сервером, но клиент об этом не узнает, потому что соединение оборвалось в момент возвращение ответа

Так какие же существуют способы для того, чтобы добиться идемпотентности в Create операциях?

1️⃣ Клиент может генерировать уникальный идентификатор Request Id (обычный UUID) для каждого запроса Create и включать его в теле запроса или заголовке. Далее сервер проверять этот идентификатор у себя в базе (можно даже кэш использовать с небольшим временем жизни) и игнорировать повторные запросы с тем же идентификатором.

2️⃣ Чем-то похож на первый вариант, но вместо создания Request id - клиент (или даже сам сервер) может генерировать уникальный идентификатор на основании выбранных полей в теле запроса. И точно также дополнительно хранить этот идентификатор у себя в базе (или кэше).

Вариантов не много (кстати, первый является более предпочтительным), потому что само по себе создание нового пораждает уникальный идентификатор, и сложно определить (а потом еще и реализовать!) было идентичное уже создано на сервере или нет.

Примеры API, как это мы обычно решаем в Google, можно глянуть в спецификации AIP-155

Еще больше про идемпотентный API можно почитать тут:
Stripe: Designing robust and predictable APIs with idempotency
Optimistic Locking in a REST API
The Amazon Builders' Library: Making retries safe with idempotent APIs

#dmdev_best_practices

Нужно закрывать ресурсы

Самый лучший вариант не забыть закрыть ресурсы после их использования - это использовать библиотеки или встроенный в язык функционал, который работает с ресурсами на уровень выше.

Другими словами говоря, если ты пишешь код так, чтобы не давать доступ пользователю к открытому ресурсу, то он попросто не сможет забыть его закрыть. Поэтому большинстве IO операций могут быть решены при помощи таких классов как ByteSource, CharSink (guava), и другие, но общий принцип таков:

File file = new File("/path/to/file");
FileSource.of(file).use(resource -> doWork(resource));

Где FileSource берет на себя работу по инициализации resource, вызывает пользовательскую функцию doWork, и после всего закрывает сам resource.

Второй отличный вариант - это конечно же блок try-with-resources (начиная с Java 7), в который можно поместить любой объект (или даже несколько!), реализующий интерфейс AutoCloseable:

try (InputStream in = Files.newInputStream(sourcePath);
     OutputStream out = Files.newOutputStream(destinationPath)) {
    doWork(in, out);
}

И опять же в методе doWork программисту не нужно думать о закрытии ресурсов, потому что они будут закрыты автоматически на уровень выше в обратном от их инициализации порядке, т.е. сначала закроется объект out, потом in. Более того, даже если произайдет ошибка при открытии потока OutputStream, первый InputStream все равно будет закрыт.

Кстати, в Java 9 добавили функционал по закрытию ресурсов, которые НЕ БЫЛИ проинициализированы в блоке try-with-resources. Тем самым дав возможность программистам закрывать ресурсы, которые были созданы на уровень выше. Поэтому советую прибегать к этому функционалу только в случае необходимости 🙂

#dmdev_best_practices

Не создавай singleton - используй Dependency Injection Framework

Для того, чтобы создать singleton в Java - есть два основных варианта:

1. Самый общеизвестный и простой (и который я показываю в своих курсах), требующий создать static final поле, хранящее единственный экземпляр класса, и private конструктор, чтобы никто не создал объект этого класса

class UserRepository {
    private static final UserRepository INSTANCE = new UserRepository();

    private UserRepository() {}

    public static UserRepository getInstance() {return INSTANCE;}
}

Но здесь появляются проблемы с десериализацией (и другие обходные пути создания более одного объекта), сложность управления зависимостями, а значит и затруднения при тестировании из-за тесной связи singleton с другими компонентами приложения.

2. Второй вариант решает часть из этих проблем - это enum. Его также советовал Джошуа Блох (Item 3), но в то же самое время добавляет другие ограничения и чувствуется "не есественность" в таком подходе. Поэтому программисты обходят его стороной.

enum UserRepository {
   INSTANCE;
}

Тем не менее, лучше избегать создания обоих вариантов синглтонов, если есть возможность использовать Dependency Injection Framework, такого как Spring или Guice. Фреймворк управляет созданием и предоставлением зависимостей для объектов в приложении, что делает код более гибким, легко поддающимся изменениям и сильно упрощается тестирование.

#dmdev_best_practices

Невозможное условие

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

Первое, что стоит попробовать - это постараться исправить код так, чтобы невозможное условие никогда не возникало!

Если код исправить не получилось, то лучше обрабатывать такие невозможные условия, вызывая AssertionError. Обычно сообщение об исключении не требуется. Но если же ты все-таки начинаешь долго думать над ним, то это признак того, что условие скорее всего не так уж и невозможно!

Кстати, если невозможное условие является исключением, полезно назвать его impossible (как в примере ниже).

// Компилятор требует обработки IOException, но мы знаем, что IO исключение невозможно при конкатенации строк.
// Кстати, это отличный вариант для использования Lombok аннотации @SneakyThrows.
Appendable a = new StringBuilder();
try {
  a.append("hello");
} catch (IOException impossible) {
  throw new AssertionError(impossible);
}

// Строка, которую невозможно достичь, только если не изменить логику кода в будущем
switch (array.length % 2) {
  case 0:
    return handleEven(array);
  case 1:
    return handleOdd(array);
  default:
    throw new AssertionError("array length is not even or odd:  " + array.length);
}

#dmdev_best_practices

Избегай громоздких списков параметров

Думаю, программистам интуитивно понятно, что сигнатуры методов (в первую очередь public) - должны быть хорошо продуманы, чтобы в последующем легко читались и использовались в коде. И особенно это касается количества параметров методов. Например, Джошуа Блох (Item 51) рекомендует не более 4, и я с ним практически согласен.

Хотя в идеале метод вообще должен состоять ровно из одного значения, которое передается на вход, и одного значения на выходе (return). А все, что происходит внутри - не должно менять каким-то образом состояние этого параметра.

Кроме количества также важен и тип параметров, ибо очень неприятно использовать методы, например, с 4 типами int. В одном из практическом видео на курсе Java Level 2 (доступно спонсорам Junior level) я демонстрировал домашнее задание с прямоугольником, в котором нужно было указать две точки для его определения:

// Вот так неудобно использовать
public Rectangle(int top, int bottom, int left, int right) {}

// А вот так и удобнее, и гораздо понятнее. Еще и запутаться сложно
public Rectangle(Point upperLeft, Point lowerRight) {}

Второй отличный вариант избегать большого количества параметров (валидно только для конструкторов) - это конечно же паттер Builder, про который мы уже обсуждали в одном из предыдущем best practice.

Третий - объединять несколько параметров в один объект Dto/Value.

#dmdev_best_practices

Предпочитай методы, реализующие функциональные интерфейсы, а не возвращающие функциональные интерфейсы

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

csvRows.stream()
    .filter(csvRowValidatorPredicate(fileContext))
    .toList();

Predicate<CsvRow> csvRowValidatorPredicate(FileContext context) {
  return csvRow -> context.findSuitableValidator(csvRow).isValid();
}

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

csvRows.stream()
    .filter(row -> isCsvRowValid(row, fileContext))
    .toList();

boolean isCsvRowValid(CsvRow row, FileContext context) {
  return context.findSuitableValidator(row).isValid();
}

Также хотелось бы добавить, что две парадигмы программирования, функциональное и объектно-ориентированное, не исключают друг друга. Каждая имеет свои плюсы и минусы: какие-то задачи лучше решать с помощью функционального стиля, какие-то с помощью объектно-ориентированного.

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

#dmdev_best_practices

Lambdas vs Method references

С предыдущего поста мы уже поняли, что лямбда-выражения лучше всего использовать для небольших и простых фрагментов кода, которые занимают в идеале 1 строчку, а максимум субъективен для каждого, но все-таки не должен превышать 3-5 строк.

С другой стороны в Java есть ссылки на метод (method reference) - это альтернативный синтаксис лямбда-выражения, который, по сути, передает параметры лямбда-выражения именованному методу.

Но когда/что лучше использовать?

В принципе, нужно склоняться в пользу method reference когда только это возможно. Ссылки на методы столь же эффективны, а иногда даже более эффективны, чем лямбда-выражения (правильнее даже сказать, что под-капотом лямбда-выражения преобразуются в ссылки на методы, чем наоборот). Особенно если лямбда-выражение становится слишком длинным - просто перенеси его тело в метод и вместо этого используй ссылку на метод.

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

// Lambda expression выглядит приятнее
.map(it -> splitToColumns(it))

// Чем аналогичный method reference
.map(UserDataCsvFileConvertorUtils::splitToColumns)

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

#dmdev_best_practices

Генерация тестовых данных

Чтобы написать хороший тест, нужны хорошие тестовые данные, приближенные к production. Плохо подготовленные данные = плохо написанный тест.

Поэтому практически все тесты должны состоять из трех основных частей:
- given (подготовка данных и стабов для mock/spy)
- when (вызов тестируемого API)
- then (проверка результата)

Сложно написать хороший тест, полагаясь на данные, которые уже существуют в базе в момент запуска теста (за исключением справочных данных или тех, что были накатаны на production с помощью миграционных фреймворков вроде liquibase и flyway). Обычно на эти данные полагаются другие тесты, а потому часто меняются, что ломает наши тесты или делает их даже flaky.

Поэтому каждый тест должен в идеале готовить данные только для себя, на которых он планирует проверить API:

@Test
void findAll() {
    // given
    // Все компактно, содержит только необходимую информацию для программиста
    User user1 = userDao.save(getUser("[email protected]"));
    User user2 = userDao.save(getUser("[email protected]"));
    User user3 = userDao.save(getUser("[email protected]"));

    // when
    List<User> actualResult = userDao.findAll();

    // then
    // Легко получить доступ к id объектов, т.к. накатывание данных было в самом тесте
    assertThat(actualResult).hasSize(3);
    List<Integer> userIds = actualResult.stream()
            .map(User::getId)
            .toList();
    assertThat(userIds).contains(user1.getId(), user2.getId(), user3.getId());
}

А чтобы не испортить состояние базы во время проверки, то:
- открываем транзакцию ПЕРЕД выполнением теста (@BeforeEach)
- накатываем данные, вызываем API и проверяем результат (@Test)
- откатываем транзакцию в конце (@AfterEach)

#dmdev_best_practices

Wildcards

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

Поэтому не стоит усложнять свой API еще больше, когда можно обойтись, например, wildcards вместо создания нового параметризованного типа:

// Wrong
// Параметр <T> просто мешает чтению сигнатуры метода и не дает никакой пользы.
public <T extends Number> void update(Collection<T> ids) { ... }

// Right
public void update(Collection<? extends Number> ids) { ... }

Ну и еще раз напомню то, что объяснял на курсе Java Level 2 (Generics), и что является ключевым в понимании параметризации и wildcars в Java - это аббревиатура PECS.
PECS - Producer: Extends; Consumer: Super

// Метод добавляет (produce) элементы в коллекцию, поэтому Extends
void addAll(Collection<? extends E> collection) {
    for (E item : c) {
        add(item);
    }
}

// Объект filter потребяет (consume) элемент из коллекции, чтобы отвалидировать его, поэтому Super
void removeIf(Predicate<? super E> filter) {
    Iterator<E> each = iterator();
    while (each.hasNext()) {
        if (filter.test(each.next())) {
            each.remove();
        }
    }
}

PS. Более подробно про Generics также можно почитать в топ 1 книге для джавистов Effective Java (Item 31)

#dmdev_best_practices

Interruption

Остановить поток выполнения - не совсем тривиальная задача. Мы не знаем, какие ресурсы были заняты, а значит и не знаем, как их освободить. Поэтому еще во 2 версии Java метод stop был deprecated. Теперь мы можем только послать "сигнал" потоку, чтобы он увидел, что мы хотим "прервать" его выполнение. Тогда он сможет спокойно завершиться и почистить все необходимые ресурсы.

Когда в принципе может понадобится это прерывание? На самом деле основных вариантов немного (но встречаются очень часто в приложениях):
- graceful shutdown (всего приложения или его части)
- cancel (пользователь хочет отменить выполнение задачи)
- timeouts (задача/запрос выполняется дольше, чем отведено на это время)
- остановить ненужные параллельные задачи

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

Поэтому я выработал для себя несколько основных правил, которым всегда следую:
1. Когда это только возможно в логике (чаще всего перед каждой итерацией цикла) - проверяй Thread.isInterrupted()
2. Если ты увидел, что нужно прервать выполнение - то делаешь весь необходимый cleanup и пробрасываешь дальше InterruptedException
3. Если пробросить InterruptedException не получается (например, нужно свое кастомное исключение), то обязательно вызови Thread.currentThread().interrupt(), чтобы восстановить interrupted status

try {
  logicThatCanBeTimeOutOrCancelled();
} catch (InterruptedException e) {
  Thread.currentThread().interrupt(); // Restore the interrupted status
  throw new MyCheckedException("describe what task was interrupted", e);
}

for (Path file : files) {
  if (Thread.isInterrupted()) { // Check interruption before each iteration
    // do cleanup and throw an InterruptedException
    throw new InterruptedException();
  }
  doExpensiveTask(file);
}

#dmdev_best_practices

Избегай длительного выполнения задач

Довольно часто на практике необходимо реализовывать задачи или scheduled jobs, которые должны долго выполнять какую-то работу. Например:
- рассылка недельных отчетов (статистика) для миллионов пользователей в системе
- обновление статуса всех expired подписок
- обработка файлов/фото/видео

Если такие задачи решать в лоб, то можно просто создать отдельный поток, который, например, начинает идти по всем пользователям в базе данных и рассылать им email.

Но какие проблемы мы получаем в таком случае?
1️⃣ Мы даже примерно не знаем, когда закончит выполнение задача. А в это время мы хотели бы рестартовать приложение, выкатывать новые версии его и т.д. - что означает прерывание задачи (что мы писали в предыдущем посте) и последующий запуск с того момента, на котором мы остановились.
2️⃣ Сложно распределить нагрузку между несколькими инстансами сервиса. Чаще всего такие задачи можно запустить только на одном, иначе возникает конфликт (который еще нужно избежать, потратив время на соответствующую реализацию!).
3️⃣ Трудоемкие долго живущие задачи могут замедлять оставшийся функционал сервиса, что негативно скажется на пользователях, а мы не сможем даже это исправить.
4️⃣ Такие задачи сложны в поддержке и обслуживании (как остановить ее? как поставить на паузу? как перезапустить? на все эти операции писать отдельный API?). Они более подвержены ошибкам и требуют больше усилий для исправления и оптимизации.

Поэтому предпочтительнее разбивать сложные задачи на более мелкие, более управляемые подзадачи
Например:
- запускать задачу гораздо чаще и обрабатывать только batch пользователей (например, по 1000)
- ограничивать по времени (например, отсылать emails не более минуты - после чего завершать работу).

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

#dmdev_best_practices

Не следует сохранять лямбда-выражения в переменных/константах

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

Function<String, CsvRow> CONVERT_TO_CSV_ROW_FUNCTION = 
   line -> new CsvRow(line.split(","));

Predicate<CsvRow> VALIDATE_CSV_ROW_PREDICATE =
   csvRow -> !csvRow.hasEmptyColumns();

return lines.stream()
    .map(CONVERT_TO_CSV_ROW_FUNCTION)
    .filter(VALIDATE_CSV_ROW_PREDICATE)
    .toList();

На самом деле, тут не все так плохо, но все-таки лучше рассматривать другой более читабельный вариант, где лямбда-выражения остаются внутри Stream API/Optional или вообще выносятся в отдельные методы:

return lines.stream()
    .map(line -> new CsvRow(line.split(",")))
    .filter(csvRow -> !csvRow.hasEmptyColumns())
    .toList();

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

И последнее:

Нет необходимости сохранять лямбда-выражения в константы, чтобы "улучшить производительность" приложения. JVM оптимизирует те участки кода, где это необходимо. И делает это очень даже умно! Лучше обратить свое внимание на IO операции: там 90%+ проблемных по перфомансу мест.

#dmdev_best_practices

Используй deadline propagation

Обычно, каждый RPC запрос должен выполняться с установленным deadline, который означает максимальное количество времени, отведенное на выполнение этого запроса. Если время превышает максимально допустимое - прерывается выполнение запроса и пробрасывается timout exception. Более того, сервер может пробросить это исключение сразу же, если видит, что deadline короче, чем требуется для выполнения запроса.

Но зачем в принципе использовать deadlines?
- чтобы не допускать запросов, которые выполняются бесконечно (особенно критично для user-facing)
- чтобы не тратить ресурсы сервера на поддержание долгих/бесконечных запросов

Зачем тогда deadline propagation?
Дело в том, что зачастую сервер не обрабатывает запрос полностью сам, а делает дополнительно другие downstream RPC запросы. И тогда становится вопрос - какой deadline устанавливать для них? Устанавливать какой-то дефолтный - плохо, он может быть меньше или больше необходимого. Поэтому deadline высчитывается из оставшегося времени, которое отвел на выполнение инициатор запроса (например, frondend).

Например, как на картинке:
- Client (инициатор) отправляет запрос с deadline = 10 секундам
- Server A обрабатывает запрос сам за 5 секунд и делает 2 асинхронных RPCs в Server B и D, установив каждому deadline = 5 секунд (10 - 5)
- Server B обрабывает запрос сам за 3 секунды и делает 1 RPC запрос в Server C, установив deadline = 2 секунды (5 - 3)

Как это работает?
- в случае синхронных серверов, вроде Apache Tomcat, то можно просто хранить значение deadline в ThreadLocal и при выполнении любой IO операции - получать ее оттуда (иначе использовать дефолтный или не ограничивать вовсе)
- в случае асинхронных серверов, вроде Netty, использовать аналог ThreadLocal - это Local Context.

#dmdev_best_practices

JUnit assertions

Во фреймворке JUnit есть свои классические методы для проверки ожидаемого (expected) и актуального (actual) значений. Но как показывает практика, довольно часто программисты делают ошибки, путая местами параметры actual и expected. Из-за этого логи могут вводишь в замешательство, почему, например, актуальное значение не верно, хотя на самом деле - оно просто попутано местами.

// Только заглянув в название параметров, можно понять где что
public static void assertEquals(Object expected, Object actual) {
 AssertEquals.assertEquals(expected, actual);
}

Именно поэтому я предпочитаю использовать дополнительные библиотеки, такие как AssertJ или Truth. Потому что я всегда знаю, что на вход каждый метод assertThat принимает именно актуальное значение. Кстати, в своем курсе JUnit 5 я более подробно рассказывал про это.

// Пример метода assertThat из библиотеки AssertJ
public static <T> ObjectAssert<T> assertThat(T actual) {
  return AssertionsForClassTypes.assertThat(actual);
}

Есть только несколько исключений, когда я могу воспользоваться классическими методами из JUnit 5, потому что они короче и проще/лучше читаются:

assertNull(Object actual);
assertNotNull(Object actual);
assertTrue(boolean condition);
assertFalse(boolean condition);

В остальных случаях все-таки не следует мешать в одном тесте сразу несколько библиотек, а предпочитать одну!

#dmdev_best_practices

Не отправляй stale notifications пользователю

На практике довольно часто приходится отправлять различные уведомления пользователю (email, in-app notifications), когда происходит какое-то событие. Например:
- у пользователя заканчивается срок действия подписки
- он пробежал свой первый марафон и нужно вручить особенный badge
- необходимо поставить электронную подпись на только что отправленные документы
- пришел счет на оплату электроэнергии
- и т.д.

И также очень часто такие уведомление отправляются асинхронно от основной логики приложения, потому что если что-то пойдет не так с отправкой - это не должно останавливать или повторять заново основную логику. Да и время не хочется тратить на ожидание того, когда закончится отправка уведомления.

Поэтому их часто складывают в какую-нибудь очередь в базе данных, или в Message Brokers вроде Kafka.

И казалось бы, что может пойти не так - но на деле сообщения из таких источников могут приходить в места их обратки с запозданием (а порой и с запозданием в несколько дней/недель, если что-то пошло не так). Получить badge об успешном прохождении марафонской дистанции через пару дней - хоть и не приятно, но еще не критично. Но вот поставить электронную подпись на документы, которые нужно было подписать еще несколько недель назад - уже более критично. Не говоря про оплату элекроэнергии за прошлый месяц, когда уже пора платить за новый и с пеней.

Поэтому прямо перед самой отправкой уведомлений нужно всегда проверять еще раз актуальность этой отправки

- действительно ли дата предполагаемой отправки не outdated?
- действительно ли событие (истекла подписка) произошло совсем недавно, а не далеко в прошлом?
- действительно ли состояние пользователя в базе данных сооветствует отправляемому уведомлению?
- и т.д.

Только благодаря таким double checks получится избежать отправки stale notifications и не вводить пользователя в заблуждение!

#dmdev_best_practices