Что я узнал после 1000 code review

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

Вот мои 3 (+1 бонусная) наиболее распространенные рекомендации по код-ревью.

Рекомендация 1: Выбрасывайте исключения, если что то идет не так

Зачастую паттерн выглядит так:

List<String> getSearchResults(...) {
  try {
    List<String> results = // make REST call to search service
    return results;
  } catch (RemoteInvocationException e) {
    return Collections.emptyList();
  }
}

Этот паттерн вызвал перебои в одном из мобильных приложений, над которыми я работал. Поиск на стороне сервера, который мы использовали, начал выбрасывать исключения. Оказалось, на серверном API приложения был некоторый код, похожий на приведенный выше. Поэтому приложение получало 200 ответ сервера и с радостью показывало пустой список для каждого поискового запроса.

Если бы вместо этого API выбросил исключение, то наши системы мониторинга сразу бы обработали его и исправили.

Существует много случаев, когда может возникнуть соблазн просто вернуть пустой объект после того, как вы поймали исключение. Примерами пустых объектов в Java являются Optional.empty(), null и пустой список. Один из случаев, где так и хочется вернуть null значение это парсинг URL. Вместо того, чтобы возвращать null, если URL-адрес не может быть получен из строки, спросите себя: «Почему URL-адрес неверен? Является ли это проблемой данных, которые мы должны исправлять на входном потоке?».

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

Рекомендация 2: Пользуйтесь наиболее специфическими типами данных

Эта рекомендация — альтернатива stringly typed programming.

Слишком часто я вижу такой код:

void doOperation(String opType, Data data); 
// where opType is "insert", "append", or "delete", this should have clearly been an enum

String fetchWebsite(String url);
// where url is "https://google.com", this should have been an URN

String parseId(Input input);
// the return type is String but ids are actually Longs like "6345789"

Использование наиболее специфического типа позволяет избежать целого ряда ошибок и в основном является основной причиной выбора языка со строковой типизацией, такого как Java.

Итак, теперь встает вопрос: как преуспевающие программисты в конечном итоге пишут плохой код со строковой типизацией? Ответ: потому что внешний мир не сильно типизирован. Есть несколько разных мест, откуда программа получает строки, например:

• параметры запроса и пути в URL-адресах
• JSON
• базы данных, которые не поддерживают перечисления
• плохо написанные библиотеки

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

// Step 1: Take a query param representing a company name / member id pair and parse it
// example: context=Pair(linkedin,456)
Pair<String, Long> companyMember = parseQueryParam("context");
// this should throw an exception if malformed

// Step 2: Do all the stuff in your application
// MOST if not all of your code should live in this area

// Step 3: Convert the parameter back into a String at the very end if necessary
String redirectLink = serializeQueryParam("context");

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

Рекомендация 3: Используйте Optional вместо Null

Одно из лучших нововведений в Java 8, — это класс Optional, который представляет собой объект, который может существовать или не существовать.

Тривиальный вопрос: какое исключение имеет свой собственный акроним? Ответ: NPE или Null Pointer Exception. Это, безусловно, самое распространенное исключение на Java, которое часто называют ошибкой в миллиард долларов.

Optional позволяет полностью убрать NPE из вашей программы. Однако он должен использоваться правильно. Вот несколько советов,
использование Optional:

• Вы не должны просто вызывать .get () в любое время, когда у вас есть Optional, чтобы использовать его, вместо этого тщательно подумайте о том случае, когда Optional отсутствует и придумайте рациональное значение по умолчанию.
• Если у вас еще нет рационального значения по умолчанию, то методы, как .map () и .flatMap () позволит отложить его выбор на потом.
• Если внешняя библиотека возвращает NULL, чтобы указать пустой кейс, сразу же оберните значение используя Optional.ofNullable (). Поверьте мне, вы поблагодарите себя позже. nulls имеют тенденцию «всплывать» внутри программ, поэтому лучше всего остановить их в источнике.
• Используйте Optional в качестве возвращаемого значения метода. Это здорово, потому что вам не нужно читать javadoc, чтобы выяснить, можно ли опустить это значение.

Бонусная рекомендация: «Unlift» методов, когда это возможно

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

// AVOID:
CompletableFuture<T> method(CompletableFuture<S> param);
// PREFER: 
T method(S param);

// AVOID:
List<T> method(List<S> param);
// PREFER:
T method(S param);

// AVOID: 
T method(A param1, B param2, Optional<C> param3);
// PREFER:
T method(A param1, B param2, C param3);
T method(A param1, B param2);
// This method is clearly doing two things, it should be two methods
// The same is true for boolean parameters

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

Почему?
1) Promise A method(Promise B param)
Это менее гибко, зато проще.

2) A method(B param).

Если у вас есть Promise B , вы можете использовать первый способ, или вы можете использовать второй способ путем «Lifting» функции с помощью .map. (т.е. promise.map(method)).

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

Мне нравится называть эту технику «unlifting», потому что она противоположна распространённому функциональному служебному методу «lift». Если переписать методы подобным образом, то они станут более гибкими и простыми при вызове.

Автор: rishat_edison

Источник