Как сделать код на стримах чище?
Странно что в 2k18 Google Java Code Convention не содержит вменяемых правил форматирования цепочек вызовов (a.k.a. method chaining), в частности, Java Stream API и создание объектов при помощи шаблона «Строитель», а индустриальный лидер IDE IntelliJ IDEA не умеет их автоматически форматировать. Предлагаю, в первую очередь для себя, сформулировать и постараться обосновать выбор стиля кодирования.
Для чего это нужно?
Конечная цель — повышение читабельности кода, уменьшение времени необходимого для понимания логики, повышение переиспользования. Что хотим видеть на выходе: ёмкий и лаконичный код, отсутствие стилистического отпечатка программиста. Дисциплинирование себя — наличие договоренности заставляет придерживаться взятых на себя обязательств.
Исходные данные
Используется Google Java Code Convention, но отступы как в Java Code Convention’99 (4 пробела) , максимум символов на строчку от 100 до 120, после 120 hardwrap (надеюсь, что объяснять необходимость лимита строки не стоит). Свободная трактовка Clean Code.
Шаблон «Строитель»
На примерах рассмотрим несколько вариантов использования цепочки вызовов. Внимательный читатель заметит, что по «Клин Коду» нужно стараться делать функции как можно меньше, и автор книги приводит в качестве единицы измерения строки. Я спешу предупредить — не стоит дословно следовать этому правилу пытаться уместить максимум возможного в каждой строчке. Если, конечно, у вы не преследуете цель напиcать «OC в 30 строк».
// Плохой код. Сложно найти интересующий (да и вообще любой) метод.
// Легко ошибиться и не проставить какое-либо значение.
private BasePosition getBasePosition(Portfolio portfolio) {
return BasePosition.builder().account("MyAccount").positionCurrency(Currency.RUB).closed(false).size(10) .openDate(LocalDateTime.now()).openPrice(BigDecimal.valueOf(10)).code("GOOG").portfolio(portfolio).build();
} // Хороший, читабельный код.
// При чтении легко найти интересующий метод
// Удобнее дебажить
private BasePosition getBasePosition(Portfolio portfolio) {
return BasePosition.builder()
.account("MyAccount")
.positionCurrency(Currency.RUB)
.closed(false)
.size(10)
.openDate(LocalDateTime.now())
.openPrice(BigDecimal.valueOf(10))
.code("GOOG")
.portfolio(portfolio)
.build();
}Стоит отметить, что компромисс между приведенными вариантами, в котором на одной строчке могут присутствовать несколько вызовов, стоит рассматривать как ошибку.
// Плохой код.
// вызов метода maxSize "замаскирован"
private BasePosition getBasePosition(Portfolio portfolio) {
return BasePosition.builder()
.account("MyAccount")
.positionCurrency(Currency.RUB)
.closed(false)
.size(10).maxSize(100)
.openDate(LocalDateTime.now())
.openPrice(BigDecimal.valueOf(10))
.code("GOOG")
.portfolio(portfolio)
.build();
}На личном опыте заметил, что часто хочется небольшую цепочку (2-3 метода билдера) записать в одну строчку, и в общем случае это даже не режет глаз. Но работает это только при следующих условиях: тривиальность кода, отсутствие левой части выражения, верхние уровни блоков кода.
// Плохо
private void initPortfolio(String name) {
if (demoMode) {
this.userDefaultPortfolio = EnterprisePortfolio.builder().name(name).positions(new ArrayList<>()).build();
}
}
// Не критично.
private Portfolio getPortfolio(String name) {
return Portfolio.builder().name(name).positions(new ArrayList<>()).build();
}
// До сих пор хорошо. Но всё равно есть вопрос..
private Portfolio getPortfolio(String name) {
return Portfolio.builder()
.name(name)
.positions(new ArrayList<>())
.build();
}Зачем классу с двумя полями билдер? Конструктор в данном случае смотрелся бы уместнее (отвечаю сам себе: остальные поля могут быть опциональными).
Stream API
Начнем с простых вариантов. В некоторых случаях стримы — это не монструозные конструкции с множеством лямбда-функций, а лаконичные цепочки вызовов, мало чем визуально отличающиеся от нескольких вызовов вложенных объектов. Но даже для них line wrapping не кажется чем-то искуственным.
//Вполне понятно что тут происходит, сойдет
private int getIntervalSum(int start, int end) {
return IntStream.range(start, end).sum();
}
// Пример чуть сложнее, но всё ещё читабельно
private DoubleStream getEvenNumbers() {
return IntStream.range(0, 1000).mapToDouble(value -> value * 2);
}
// ОК, метод порождающий, изначальный стрим остался в "общей" строчке
private DoubleStream getEvenNumbers() {
return IntStream.range(0, 1000)
.mapToDouble(value -> value * 2);
}
// ОК, вынесли вызов порождающего стрим метода на новую строчку
private int getPlayerLevel(int lastPlayerScore) {
return IntStream
.range(MIN_REWARD_POINTS, lastPlayerScore)
.filter(value -> value % NEXT_SCORE_LEVEL == 0)
.count();
}Теперь посмотрим на стрим коллекций, то, с чем чаще всего сталкиваемся при реализации бизнес-логики. Итеративно будем его улучшать.
// Плохой код, видно что был написан или "на отвали" или в сумасшедшем темпе
private void printActiveEmployeeNames(Collection<Entity> entities) {
printer.print(entities.stream().filter((entity) -> { return entity.isActive();})
.map((entity) -> {return entity.getFirstName() + " " + entity.getLastName()}).collect(Collectors.toList()));
}
// 1. Расставили разрывы строк, выделили отдельную переменную, но всё ещё есть над чем поработать
// 2. Убрали скобки () вокруг единичных параметров
// 3. Убрали скобки {} блоков кода и return у однострочных лямбд
private void printActiveEmployeeNames(Collection<Entity> entities) {
Collection<String> names = entities.stream()
.filter(entity -> entity.isActive())
.map(entity -> entity.getFirstName() + " " + entity.getLastName())
.collect(Collectors.toList());
printer.print(names);
}
//ОК, ёмко и лаконично
private void printActiveEmployeeNames(Collection<Entity> entities) {
Collection<String> names = entities.stream()
.filter(Entity::isActive) // Предпочтительно заменять на method reference
.map(this::getFullName) // Нетривиальные лямбды выносить в отдельные методы
.collect(toList()); // Статический импорт сократит объем лишнего кода, уберет визуальный шум
printer.print(names);
}Подытожим общие стилистические правила:
- на одной строчке должен присутствовать вызов одного метода стрима;
- нетривиальные лямбда-функции следует выносить в отдельные методы;
- тривиальные, однострочные лямбда-функции не оборачивать в скобки {}, как и одиночные параметры;
- не следует явно указывать типы параметров лямбда-функций;
- следует использовать статические импорты стандартных утилитарных методов (например, Collectors), с этим осторожнее: читаемость кода понижаться не должна, статически импортированные методы должны однозначно интерпретироваться, не должно происходить засорение пространства имен;
- кастомные и часто используемые утилитарные функции можно и нужно выносить в отдельные методы и/или классы, не забывая давать им осмысленные имена;
- идеальная лямбда — однострочная. И к этому нужно стремиться можно заменять многострочные лямбды методами (хорошая статья на эту тему). Особенно это касается блоков кода с более чем 2-3 выражениями;
- ссылки на методы предпочтительнее лямбда-функций — они повышают читабельность кода (но не всегда) и более производительны;
- по возможности нужно стараться переиспользовать лямбда-функции;
Исключения по разрыву строк возможны, если в выражении отсутствует левая часть:
// в плане разрывов допустимо
private boolean isNameAlreadyExists(String name) {
return entities.stream().anyMatch(entity-> name.equalsIgnoreCase(entity.getName()));
}
// и всё же такой вариант читается легче и быстрее
private boolean isNameAlreadyExists(String name) {
return entities.stream()
.map(Entity::getName)
.anyMatch(name::equalsIgnoreCase);
}Или если стрим тривиальный, то допускаются оба варианта:
private void filterOnlyItemsOnSale() {
items = items.stream().filter(ProductItem::isOnSale).collect(toList());
}
private void filterOnlyItemsOnSale() {
items = items.stream()
.filter(ProductItem::isOnSale)
.collect(toList());
}В принципе, похожие правила можно применить и для Optional. В чём можно убедиться на примерах
// Неэстетично оформленная цепочка, чтобы понять что происходит хотя бы в общих чертах нужно прочитать всю цепочку целиком
Optional<String> description = findEntity(id).filter(Entity::isActive).map(Entity::getSubEntity)
.filter(SubEntity::isActive).map(SubEntity::getDescription);
String description = findEntity(id).filter(Entity::isActive).map(Entity::getSubEntity)
.map(SubEntity::getDescription).orElse("Unknown");
// Достаточно одного взгляда на выстроенный код и общий смысл уже становится понятным
Optional<String> description = findEntity(id)
.filter(Entity::isActive)
.map(Entity::getSubEntity)
.filter(SubEntity::isActive)
.map(SubEntity::getDescription);
String description = findEntity(id)
.filter(Entity::isActive)
.map(Entity::getSubEntity)
.map(SubEntity::getDescription)
.orElse("Unknown");Дополнительно хотелось бы отметить моменты (по большей части это принципы, являющиеся де-факто стандартами разработки с использованием Stream API), не совсем подходящие для темы, но так или иначе касающиеся понимания кода. Основные принципы:
- Не изменять внутреннее состояние объектов стрима. Вместо этого можно возвращать копии объектов .map(entity -> entyty.toBuilder().price(10).build()) или
.map(String::toUpperCase); - Нельзя допускать побочных эффектов, иными словами запрещено использовать методы стрима не по назначению. Например, в методе filter наполнять какую-либо коллекцию или (см. п. 1) — модифицировать данные стрима;
- Не передавайть и не принимайть стримы в качестве аргументов методов и не использовать их в полях класса. Стрим штука одноразовая, при попытке повторного использования бросается java.lang.IllegalStateException.
- list.stream().forEach() -> list.forEach()
- Избегать использования каскадных лямбда-функций [a -> b -> b > a]
- Лямбда-функция не может пробросить проверяемое исключение. Следует использовать (статические утилитарные) методы, для оборачивания исключений функций, их обработки и/или бросания Unchecked-исключений.
Комментариев нет