What I Learned from Doing 1,000 Code Reviews: Common Mistakes and Best Practices

Code Review is a crucial practice for ensuring code quality. Steven Heidel, who performed code reviews at LinkedIn, summarizes common coding problems and proposes solutions.

During his time at LinkedIn, a large part of his work involved code reviews, where he observed recurring mistakes that he compiled and shared with his team.

Experience 1: Throw an exception when an error occurs

A typical pattern he saw was returning empty results instead of propagating the exception:

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

This approach hides backend failures, causing the front‑end to receive successful HTTP 200 responses with empty data, making the issue hard to detect. Throwing the exception would allow monitoring systems to catch the problem early.

Returning empty objects (e.g., Optional.empty(), null, empty list) is often inappropriate, especially for URL parsing errors; instead, the code should throw an exception.

Experience 2: Use the most specific type possible

Using generic stringly‑typed parameters leads to bugs. Examples include:

void doOperation(String opType, Data data); // should be an enum
String fetchWebsite(String url); // should be a URN
String parseId(Input input); // should return Long

Specific types prevent many bugs. The prevalence of stringly‑typed code stems from external sources such as URL query parameters, JSON, databases without enums, and poorly designed libraries. The recommended strategy is to parse and serialize strings at the program’s boundaries.

// Step 1: Parse a query param into a strongly‑typed object
Pair<String, Long> companyMember = parseQueryParam("context"); // throws if malformed
// Step 2: Core application logic works with the typed data
// Step 3: Serialize back to a string only when needed
String redirectLink = serializeQueryParam("context");

This approach catches malformed data early, reduces repeated error handling, and makes method signatures more descriptive.

Experience 3: Prefer Optionals over nulls

Java 8 introduced Optional, which can eliminate the ubiquitous NullPointerException (the most common Java exception). Proper use includes avoiding .get() without checking, providing sensible defaults, wrapping external nulls with Optional.ofNullable(), and returning Optional from methods to make absence explicit.

Additional Advice: Apply "Unlift" techniques

Avoid methods that accept container types (e.g., CompletableFuture<T>, List<T>, Optional<C>) as parameters or return the same container type. Prefer simple signatures like T method(S param) to increase flexibility and readability.

// AVOID:
CompletableFuture<T> method(CompletableFuture<S> param);
List<T> method(List<S> param);
T method(A param1, B param2, Optional<C> param3);
// PREFER:
T method(S param);
T method(A param1, B param2, C param3);

These container‑heavy signatures reduce flexibility; using plain types (the "unlift" approach) makes methods easier to use and test.

Original source: https://hackernoon.com/what-i-learned-from-doing-1000-code-reviews-fe28d4d11c71