Coffee break #71. Best Practices for Java Code Analysis

Source: DeepSource Having another pair of eyes to review code is always helpful. You don't need to be an expert to analyze someone's code. Code reviews can be done even with little experience. You just need to know how and what exactly you want to check.

1. Follow the Java coding convention

Compliance with Java code conventions helps to quickly review the code and understand it. For example, all package names in Java are written in lowercase letters, constants are in capital letters only, variable names are in CamelCase, etc. A full list of conventions can be found here . Some teams develop their own style conventions, so keep that in mind!

2. Replace imperative code with lambdas and streams

If you're using Java 8+, replacing loops and overly verbose methods with streams and lambdas will make your code cleaner. Lambdas and streams allow you to write functional code in Java. The following code snippet filters odd numbers in the traditional imperative way:

List<Integer> oddNumbers = new ArrayList<>();
for (Integer number : Arrays.asList(1, 2, 3, 4, 5, 6)) {
	if (number % 2 != 0) {
	  oddNumbers.add(number);
  }
}

And this is a functional way to filter odd numbers:

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

3. Beware of NullPointerException

When writing new methods, try to avoid returning null values ​​whenever possible. This may result in a null pointer exception. In the snippet below, the outer method returns null if there are no integers in the list.

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;
	}
}

Before directly calling an object method, I recommend checking for null, as shown below.

Items items = new Items(Collections.emptyList());
Integer item = items.highest();
boolean isEven = item % 2 == 0; // throws NullPointerException ❌
boolean isEven = item != null && item % 2 == 0  // ✅

However, having null checks everywhere can be quite cumbersome. If you're using Java 8 or higher, consider using the Optional class to represent values ​​without valid states. It makes it easy to define alternative behavior and is useful for method binding. In the snippet below, we use the Java Stream API to find the largest number with a method that returns an Optional . Note that we are using Stream.reduce , which returns an Optional value .

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;
});

Alternatively, you can also use annotations such as @Nullable or @NonNull , which will warn you if a null conflict occurs while generating your code. For example, passing a @Nullable argument to a method that takes @NonNull parameters .

4. Direct assignment to the reference field from the client code

References provided to client code can be changed even if the field is final. Let's understand this better with an example.

private final List<Integer> items;
public Items(List<Integer> items) {
        this.items = items;
}

In the snippet above, we are directly assigning a reference to the field from the client code. The client can easily change the contents of the list and manipulate our code as shown below.

List<Integer> numbers = new ArrayList<>();
Items items = new Items(numbers);
numbers.add(1); // This will change how items behaves as well

Instead, consider cloning the link, or creating a new link and then assigning it to a field, as shown below:

private final List<Integer> items;
public Items(List<Integer> items) {
    this.items = new ArrayList<>(items);
}

The same rule applies when returning references. You must be careful not to expose the internal mutable state.

5. Handle Exceptions Carefully

When catching exceptions, if you have multiple catch blocks, make sure the sequence of catch blocks is from more specific to less specific. In the snippet below, the exception will never be caught in the second block because the Exception class is the main one.

try {
	stack.pop();
} catch (Exception exception) {
	// handle exception
} catch (StackEmptyException exception) {
	// handle exception
}

If the situation is fixable and can be handled by the client (the user of your library or code), then it's better to use checked exceptions. For example IOException . It forces the client to handle the script, and in case the client decides to rethrow the exception, it should be a conscious call to ignore the exception.

6. Consider your choice of data structures

Java collections include ArrayList , LinkedList , Vector , Stack , HashSet , HashMap , Hashtable . It is important to understand the pros and cons of each in order to use them in the right context. A few tips to help you make the right choice:

• Map : Useful if you have unordered elements, keys, value pairs, and need efficient extraction, insertion, and deletion operations. HashMap , Hashtable , LinkedHashMap are implementations of the Map interface.
• List : Very often used to create an ordered list of elements. This list may contain duplicates. ArrayList is an implementation of the List interface . A list can be made thread-safe using Collections.synchronizedList . Thus, there is no need to use Vector . Here's some more information on why Vector is essentially deprecated.
• Set : Similar to List , but does not allow duplicates. Implements HashSet in the Set interface .

7. Think Twice Before “Revealing”

Java has several access modifiers - public , protected , private . If you don't want to expose the method to client code, you can leave everything private by default. Once you open the API, there is no going back. For example, you have a Library class that has a book by name check method:

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

private searchByTitle(List<Book> availableBooks, String bookName) {
...
}

If you don't keep the searchByTitle method private by default , it will eventually become available, and other classes can start using it and building on it for logic that you might want to make part of the Library class . This can break the encapsulation of the Library class and make it impossible to rollback/modify without breaking someone else's code.

8. Code for interfaces

If you have specific implementations of certain interfaces (such as ArrayList or LinkedList ) and if you use them directly in your code, this can lead to high coupling. By using the List interface , you can switch to the implementation at any point in the future without breaking your code.

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

new Bill(new ConsolePrinter());
new Bill(new HTMLPrinter());

In the snippet above, using the Printer interface allows the developer to navigate to another specific HTMLPrinter class .

9. Don't push interfaces

Take a look at the following interface:

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

class BookServiceImpl implements BookService {
...

Is there any benefit in creating such an interface? Is it possible to implement this interface as another class? Is this interface generic enough to be implemented by another class? If the answer to all these questions is no, then I definitely recommend avoiding this unnecessary interface that you will have to maintain in the future. Martin Fowler explains it very well in his blog . Well, then how to successfully use interfaces? Let's say we have a Rectangle class and a Circle class that have a behavior for calculating the perimeter. If there is a requirement that the perimeter of all shapes is a use case for polymorphism, then having an interface would make more sense:

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. Replace hashCode when overriding Equals

Objects that are equal in their values ​​are called value objects: for example, money, time. Such classes must be overridden by the equals method to return true if the values ​​match. The quals method is commonly used by other libraries for comparison and equality testing; hence the redefinition of equals is necessary. Each Java object also has a hash code value that distinguishes it from another object.

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;
    }
}

In the example above, we have overridden the equals method on 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?

We are waiting for coinCount to update the number of one rupee coins to seven since we are overriding equality. But HashMap internally checks if the hash code for two objects is equal, and only then proceeds to check for equality using the equals method . Two different objects may or may not have the same hash code, but two equal objects must always have the same hash code, as defined in the hashCode method contract . So checking the hash code in the first place is an early exit condition. This means that, like equals , the hashCode methods must be overridden to express equality.