What I learned from doing 1000 code reviews

While working at LinkedIn a large part of my job involved doing code reviews. There were certain suggestions that kept coming up over and over again, so I decided to put together a list that I shared with the team.

Here are my 3 (+1 bonus) most common code review suggestions.

Suggestion 1: Throw an exception when things go wrong

A common pattern I have seen is this:

This pattern actually caused an outage in one of the mobile apps I worked on. The search backend we were using started throwing exceptions. However, the app’s API server had some code similar to the above. Therefore, from the perspective of the app it was getting a 200 successful response and happily showed an empty list for every single search query.

If instead the API had thrown an exception, then our monitoring systems would have picked this up immediately and it would have been fixed.

There are lots of times when it may be tempting to just return an empty object after you’ve caught an exception. Examples of empty objects in Java are Optional.empty(), null, and empty list. A place where this comes up all the time is in URL parsing. Instead of returning null if the URL can’t be parsed from a String, ask yourself: “Why is the URL malformed? Is this a data issue we should be fixing upstream?”.

Empty objects are not the right tool for this job. If something is exceptional you should throw an exception.

Suggestion 2: Use the most specific type possible

This suggestion is basically the opposite of stringly typed programming.

Too often I see code like these examples: