Code reviews: How to make it work for you

GoHealth builds its own proprietary technology to support the health insurance enrollment process.

As a team, we operate business-critical software systems. Outages clearly have the potential to adversely impact financial results of any company. Hence, we put the utmost care into the quality, reliability, resilience, and maintainability of our systems. Utmost care is put into quality and maintainability. The same reasons dictate the need to invest significantly in proper architectural design of solutions for new functionalities as well as activities to assure software quality.

Peer reviews

In general, we do not use feature branches, we converge around just a single main branch. This strategy keeps the artifacts with contributions of different teams tested together and reliably deployed to production. To assure reliable releases of new features, we use feature flags to switch on / off new features, which also facilitates independent testing when such a need arises.

Reviews are done before the contribution is merged into the main branch. Code review focuses on:

  • Consistency
  • Simplicity
  • Usage of established coding style (streams, reactive asynchronous processing, functions)
  • Readability: Are the implementation and underlying algorithm understandable without deep study? Explanatory names of variables, methods, classes that “tell the story”. Plain text explanation in code doc as necessary
  • Strongly typed value objects

Peer reviews help new joiners to understand established patterns and solutions in code, and thus facilitate faster adaptation. Naturally, review comments may trigger discussion as the code review may reveal additional concerns, scope, or issues with the design. In such cases, we tend to move the discussion to a peer to peer or group conversation — whichever is faster and more productive.

Finally, we use peer reviews (in lack of time without notes) to keep the developers in different team(s) informed about ongoing changes in different subsystems to keep the mental model in their minds updated as soon as such change occurs (while documentation for further adoption follows).

©GoHealth

What is a good code?

Writing code is like building an airplane. Despite the power of the engine(s) — weight matters. Code is not for free, each word in code, each abstraction has its price. The value of the code must be higher than the price of the code.

Zero value code

Maybe value in the future

Of course, we rejuvenate the technology ecosystem together with the implementation of new functionalities and replace deprecated technologies; we still minimize the application domain code to the minimum necessary size.

Small (and simple), is beautiful

Simplification

Method code complexity

Traditional measurement is cyclomatic complexity — the number of different paths through the code in a method or function.

Object-oriented code

Object-oriented abstractions increase mental complexity by a level of magnitude. A simpler solution is preferable: functional constructs and naive value objects.

Interfaces

The complexity that exceeds one file is the implementation of the unnecessary interface(s). Traditional OO design, and also the abstraction paradigm brought by Spring framework suggests to “automatically” create interfaces for each active object/bean (object with functionality). This practice simplifies writing unit tests, but in many cases, the interface — implementation separation is never used — so the interface as an unnecessary abstraction may be deleted. Java has a good object-oriented foundation. So, of course, there are valid use scenarios for interfaces — i.e. interface in separate modules that simplify the construction of the application from multiple modules, and the dependency management between modules. Functional way of writing code — use of functional interfaces defines the service contract. Functional interfaces bring the principle of single responsibility — one reason to change class to extreme and so simplifies the code. The single responsibility of the service interface (“service layer”) is to dispatch to an operation function and to hold the transaction annotation.

Inheritance

Inheritance makes it hard to find the code that is executed — maybe it is in parent hierarchy, maybe overwritten. In Java, it is prohibited to make multi-parent inheritance. Deep inheritance (more than 2 levels) makes the code “magical”, full understanding requires browsing over many files and returning back. By constructing complex classes we prefer composition over inheritance. Simple classes without interfaces and inheritance localize the whole domain logic into one file, and utility and data provider classes hide their complexity as black boxes. Inheritance has its place in a well-designed model, or in JPA though for shared definitions in symmetric/derived models.

Generics

Generics are a powerful construct for expressing universal algorithms. However, if used in application domain logic, its usage can abruptly lead to unnecessary complexity. The combination of generics with inheritance is a very attractive secret sauce for writing very complicated classes, very hard to understand.

Design patterns

It is a fundamental programming skill to know all the basic design patterns. In some cases, usage of design patterns is a symptom of overengineering and may build code that is hard to extend. An example of an unnecessarily increasing complexity is the visitor pattern. In many cases, the functionality may be implemented with a naive direct solution — simple but not simplistic.
Of course, it is not a strict rule, design patterns are a good tool, but have to be used with caution in the right place.

©GoHealth

Code for people

It is helpful to split complex boolean logic and complex calculations into local variables, hence “name” the sub-values, explain the calculations. The names are a good way to document the code and help the flow of reading, that is not distracted by comments.

JavaDoc is the traditional promise to provide code as documentation. If JavaDoc is mandatory, required by build configuration settings, I prefer to generate JavaDoc comments with a sophisticated IDE plugin. The plugin tries to interpret the name of the class or method, the name of the returned class, and the parameters to generate a meaningful description. On one side it is good proof that the names are well-shaped also for human readers, on the other side the comment is good enough for elementary cases and is a good frame for additional description of the algorithm or method functionality.

We prefer long descriptive class (function) names. Long enough to be unambiguous and describe the single responsibility, but short enough that there are redundant or duplicate words. A hard balance, but worth the effort, esp. given people don’t like writing good docs.

How much to invest in a code review?

Code reviews are not for free. The expectation is not to find everything that could be written better. We involve more developers in each review — more eyes notice more, each programmer has a different experience, a different perspective, and focus. The severity of found problems exponentially shrinks with time. And we have to find the optimal time invested in a review that gives valuable findings.

Written by Tomi Vanek

Follow us on our web and social media!
GoHealth.sk | Facebook | Instagram | LinkedIn | YouTube

Leading U.S. healthtech provider now located also in Slovakia. Modern technology, functionality and exciting projects are what motivate us.