Источник: DeepSource Наличие еще одной пары глаз для проверки кода всегда полезно. Вам не нужно быть экспертом, чтобы проанализировать чей-то код. Ревью кода можно проводить даже с небольшим опытом работы. Нужно лишь знать, как и что именно вы хотите проверить.Кофе-брейк #71. Рекомендации по анализу Java-кода - 1

1. Следуйте соглашению по оформлению кода Java

Соблюдение Java code conventions помогает быстро просмотреть код и понять его. Например, все имена пакетов в Java пишутся строчными буквами, константы — только заглавными, имена переменных — в CamelCase и т. д. Полный список соглашений можно найти здесь. Некоторые команды разрабатывают собственные соглашения о стиле, так что учтите это!

2. Замените императивный код лямбдами и потоками

Если вы используете Java 8+, замена циклов и слишком подробных методов потоками и лямбда-выражениями сделает код более чистым. Лямбды и потоки позволяют писать функциональный код на Java. Следующий фрагмент кода фильтрует нечетные числа традиционным императивным способом:

List<Integer> oddNumbers = new ArrayList<>();
for (Integer number : Arrays.asList(1, 2, 3, 4, 5, 6)) {
	if (number % 2 != 0) {
	  oddNumbers.add(number);
  }
}
А это функциональный способ фильтрации нечетных чисел:

List<Integer> oddNumbers = Stream.of(1, 2, 3, 4, 5, 6)
  .filter(number -> number % 2 != 0)
  .collect(Collectors.toList());

3. Остерегайтесь NullPointerException

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

class Items {
	private final List<Integer> items;
	public Items(List<Integer> items) {
	        this.items = items;
	}
	public Integer highest() {
	  if (items.isEmpty()) return null;
	  Integer highest = null;
	  for (Integer item : items) {
	      if (items.indexOf(item) == 0) highest = item;
	      else highest = highest > item ? highest : item;
	  }
	  return highest;
	}
}
Перед прямым вызовом метода объекта я рекомендую проверить наличие null, как показано ниже.

Items items = new Items(Collections.emptyList());
Integer item = items.highest();
boolean isEven = item % 2 == 0; // throws NullPointerException ❌
boolean isEven = item != null && item % 2 == 0  // ✅
Однако иметь повсюду проверки на null может быть довольно обременительно. Если вы используете версию Java 8 или выше, рассмотрите возможность использования класса Optional для представления значений без действительных состояний (valid states). Он позволяет легко определять альтернативное поведение и полезен для связывания методов. В приведенном ниже фрагменте мы используем Java Stream API, чтобы найти наибольшее число с помощью метода, возвращающего Optional. Обратите внимание, что мы используем Stream.reduce, который возвращает значение Optional.

public Optional<Integer> highest() {
    return items
            .stream()
            .reduce((integer, integer2) > 
							integer > integer2 ? integer : integer2);
}
Items items = new Items(Collections.emptyList());
items.highest().ifPresent(integer -> {             // ✅
    boolean isEven = integer % 2 == 0;
});
В качестве альтернативы вы также можете использовать аннотации, такие как @Nullable или @NonNull, которые предупредят вас, если при создании кода возникнет конфликт null. Например, передача аргумента @Nullable методу, принимающему параметры @NonNull.

4. Прямое присвоение полю ссылок из кода клиента

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

private final List<Integer> items;
public Items(List<Integer> items) {
        this.items = items;
}
В приведенном выше фрагменте мы напрямую назначаем полю ссылку из клиентского кода. Клиент может легко изменять содержимое списка и управлять нашим кодом, как показано ниже.

List<Integer> numbers = new ArrayList<>();
Items items = new Items(numbers);
numbers.add(1); // This will change how items behaves as well
Вместо этого рассмотрите возможность клонирования ссылки или создания новой ссылки, а затем назначения ее полю, как показано ниже:

private final List<Integer> items;
public Items(List<Integer> items) {
    this.items = new ArrayList<>(items);
}
То же правило применяется при возврате ссылок. Вы должны быть осторожны, чтобы не раскрыть внутреннее изменяемое состояние.

5. Осторожно обращайтесь с исключениями

При перехвате исключений, если у вас есть несколько блоков перехвата, убедитесь, что последовательность блоков перехвата — от более конкретных к менее. В приведенном ниже фрагменте исключение никогда не будет перехвачено во втором блоке, поскольку класс Exception — главный.

try {
	stack.pop();
} catch (Exception exception) {
	// handle exception
} catch (StackEmptyException exception) {
	// handle exception
}
Если ситуация исправима и может быть обработана клиентом (пользователем вашей библиотеки или кода), тогда лучше использовать проверенные исключения. Например IOException. Оно заставляет клиента обрабатывать сценарий, и в случае, если клиент решает повторно выбросить исключение, это должен быть сознательный призыв игнорировать исключение.

6. Обдумайте выбор структур данных

В Java коллекции входят ArrayList, LinkedList, Vector, Stack, HashSet, HashMap, Hashtable. Важно понимать плюсы и минусы каждого из них, чтобы использовать их в правильном контексте. Несколько советов, которые помогут сделать правильный выбор:
  • Map: Полезен, если у вас есть неупорядоченные элементы, ключи, пары значений, и требуются эффективные операции извлечения, вставки и удаления. HashMap, Hashtable, LinkedHashMap — это варианты реализации интерфейса Map.
  • List: Очень часто используется для создания упорядоченного списка элементов. Этот список может содержать дубликаты. ArrayList — это реализация интерфейса List. Список можно сделать потокобезопасным, используя Collections.synchronizedList. Таким образом, отпадает необходимость в использовании Vector. Вот еще немного информации о том, почему Vector по сути устарел.
  • Set: Аналогичен List, но не допускает дубликатов. Реализует HashSet в интерфейсе Set.

7. Подумайте дважды перед “раскрытием”

В Java есть несколько модификаторов доступа — public, protected, private. Если вы не хотите предоставлять метод клиентскому коду, можете оставить всё private по умолчанию. После того, как вы откроете API, дороги назад уже не будет. Например, у вас есть класс Library, в котором есть метод проверки book by name:

public checkout(String bookName) {
	Book book = searchByTitle(availableBooks, bookName);
  availableBooks.remove(book);
  checkedOutBooks.add(book);
}

private searchByTitle(List<Book> availableBooks, String bookName) {
...
}
Если вы не сохраните метод searchByTitle по умолчанию как private, то в конечном итоге он станет доступным, и другие классы могут начать использовать его и строить на его основе логику, которую вы, возможно, хотели бы сделать частью класса Library. Это может нарушить инкапсуляцию класса Library и сделает невозможным откат / изменение без нарушения чужого кода.

8. Код для интерфейсов

Если у вас есть конкретные реализации определенных интерфейсов (например, ArrayList или LinkedList), и если вы используете их непосредственно в своем коде, это может привести к усилению связи (high coupling). Используя интерфейс List, вы можете переключиться на реализацию в любой момент в будущем, не нарушая код.

public Bill(Printer printer) {
	this.printer = printer;
}

new Bill(new ConsolePrinter());
new Bill(new HTMLPrinter());
В приведенном выше фрагменте использование интерфейса Printer позволяет разработчику перейти к другому конкретному классу HTMLPrinter.

9. Не “насаждайте” интерфейсы

Взгляните на следующий интерфейс:

interface BookService {
		List<Book> fetchBooks();
    void saveBooks(List<Book> books);
    void order(OrderDetails orderDetails) throws BookNotFoundException, BookUnavailableException;	
}

class BookServiceImpl implements BookService {
...
Есть ли польза от создания такого интерфейса? Есть ли возможность для реализации этого интерфейса в виде другого класса? Достаточно ли универсален этот интерфейс для реализации другим классом? Если ответ на все эти вопросы отрицательный, то я определенно рекомендую избегать этого ненужного интерфейса, который вам придется поддерживать в будущем. Мартин Фаулер очень хорошо объясняет это в своем блоге. Что ж, тогда как удачно использовать интерфейсы? Допустим, у нас есть класс Rectangle и класс Circle, у которых есть поведение для вычисления периметра. Если есть требование, что периметр всех форм — это вариант использования полиморфизма, то наличие интерфейса будет иметь больше смысла:

interface Shape {
		Double perimeter();
}

class Rectangle implements Shape {
//data members and constructors
    @Override
    public Double perimeter() {
        return 2 * (this.length + this.breadth);
    }
}

class Circle implements Shape {
//data members and constructors
    @Override
    public Double perimeter() {
        return 2 * Math.PI * (this.radius);
    }
}

public double totalPerimeter(List<Shape> shapes) {
	return shapes.stream()
               .map(Shape::perimeter)
               .reduce((a, b) -> Double.sum(a, b))
               .orElseGet(() -> (double) 0);
}

10. Замените hashCode при переопределении Equals

Объекты, которые равны по своим значениям, называются объектами значений (value objects): например, деньги, время. Такие классы должны переопределяться методом equals, чтобы возвращать true, если значения совпадают. Метод quals обычно используется другими библиотеками для сравнения и проверки равенства; следовательно, переопределение equals необходимо. Каждый объект Java также имеет значение хэш-кода, которое отличает его от другого объекта.

class Coin {
    private final int value;

    Coin(int value) {
        this.value = value;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Coin coin = (Coin) o;
        return value == coin.value;
    }
}
В приведенном выше примере мы переопределили метод equals для Object.

HashMap<Coin, Integer> coinCount = new HashMap<Coin, Integer>() {{
  put(new Coin(1), 5);
  put(new Coin(5), 2);
}};

//update count for 1 rupee coin
coinCount.put(new Coin(1), 7);

coinCount.size(); // 3 🤯 why? 
Мы ожидаем coinCount, чтобы обновить количество монет по одной рупии до семи, поскольку мы переопределяем равенство. Но HashMap внутренне проверяет, равен ли хэш-код для двух объектов, и только затем переходит к проверке равенства с помощью метода equals. Два разных объекта могут иметь или не иметь один и тот же хэш-код, но два равных объекта всегда должны иметь один и тот же хэш-код, как определено в контракте метода hashCode. Таким образом, проверка хэш-кода в первую очередь является ранним условием выхода. Это означает, что как и equals, методы hashCode должны быть переопределены, чтобы выразить равенство.