Mastering Code Reviews | Article 3: Hunting “Invisible” Bugs — Following the Data
Most junior devs review code by checking if the logic “looks” right. They look at a for loop or an if statement and say, “Yeah, that works.”
But in a complex Java system, the worst bugs aren’t in the math; they are in the state. As a seasoned engineer, I’ve learned to ignore the syntax and trace the data flow. I want to see if the data stays sane as it moves through the system.
Here are the three “invisible” things I hunt for in every Java PR:
1. The “Mutating State” Nightmare
Java makes it easy to change an object by accident. If a dev passes a List into a method and that method calls .add() or .clear(), you’ve just created a side effect.
The Senior Check:
I look for where objects are being modified—especially when those objects are passed around across multiple parts of the system.
The Red Flag:
Methods that change input parameters directly instead of returning new, modified values.
The Fix:
I push for immutability. When you make objects immutable, you prevent the risk of accidental changes that can be difficult to track. Immutability makes code easier to reason about because it guarantees that once an object is created, it won’t change. This is critical when you’re debugging or tracing a bug at 3:00 AM—you know the data will remain as it was originally passed.
2. The “Thread-Safety” Trap
In a Spring Boot or Micronaut app, your beans are typically Singletons. This means they are shared across multiple users. But shared state can be a major risk when handling concurrent requests.
The Senior Check:
I pay special attention to instance variables that aren’t final because they can lead to race conditions in multi-threaded environments.
The Red Flag:
Mutable state shared across threads, such as counters or non-final fields in a service class. If two users hit that service simultaneously, their data can overlap and lead to unpredictable behavior.
The Disaster:
Concurrency bugs often won’t show up in development, but once they hit production, they can lead to data corruptionand system crashes. If your service is mutating state across multiple threads without any safeguards, you’re opening yourself up to huge problems.
I always favor final fields and thread-safe designs, so data can’t be overwritten or shared improperly between threads. If two threads access the same resource at once, there should be no risk of inconsistent data.
3. The “Stream” and “Optional” Abuse
Java 8+ introduced powerful features like Streams and Optionals, but they are easy to misuse, especially when developers don’t fully understand their behavior.
The Senior Check:
I look for lazy mistakes—where an operation appears efficient on the surface, but ends up causing hidden issues later.
The Red Flags:
- Accessing
Optionalvalues without checking.isPresent(). This is a classic case of NullPointerExceptionwaiting to happen. - Doing heavy operations like database queries or API calls inside a stream operation. These calls are lazy, meaning they don’t execute until a terminal operation happens, but doing this can unintentionally perform many more operations than expected.
These features are meant to simplify code, but misusing them can lead to serious performance and stability issues. It’s essential to carefully consider how you’re handling optional values and streaming data, especially if you’re making external calls that could slow down the system.
How to “Trace” the Data
When reviewing a PR, I don’t just look at the logic or syntax. I choose one variable—usually the input—and follow it through the entire flow of the code.
Entry:
Where does the data come from? Is it validated before it enters the system?
Transformation:
Does the method modify the original object, or does it return a copy with the desired changes?
Exit:
Where does the data go next? Does it reach the database, another API, or the UI?
If the data flows in an unexpected way or changes in a way it shouldn’t, I’ll block the PR. Ensuring that the data is consistent and well-tracked as it flows through the system is the key to avoiding many of the bugs that surface only in production.
The Bottom Line
Don’t just read the logic; follow the data. If you can’t clearly see how an object changes from the start of the PR to the end, the code is too complex, and it will break.
In the world of Java, maintaining the integrity of data flow throughout the system is one of the most important parts of writing robust and maintainable code. By carefully reviewing how data is passed, transformed, and handled, you can avoid many of the invisible bugs that only reveal themselves under stress.
In the next article, we’ll dive into the “3:00 AM Test”. I’ll show you how I review for Production Readiness—because code that works isn’t always code that is supportable.