Every Java developer steps on the same set of landmines. A null reference, an Integer comparison that lies, an int that quietly overflows, a BigDecimal that should have been used three lines earlier. This lesson walks through the pitfalls that show up most often in real code, shows the buggy version, explains why it bites, and then shows the fix.

NullPointerException is the most common runtime crash in Java. The cause is always the same: code dereferences a variable that holds null. The variety comes from where the null shows up.

Chained Method Calls

A long chain of method calls is one null away from disaster.

What's wrong with this code?

The customer was constructed with a null address. The chain customer.getAddress().getCity().toUpperCase() calls getAddress() first, which returns null, and the next .getCity() throws. Each method in the chain is a separate place where null can sneak in.

Fix:

Optional carries the "might be null" intent through the chain. Each map only runs if the previous value was present. If you don't want Optional, write explicit null checks at each step. The one thing you cannot do is hope.

Map.get Returning Null

Map.get returns null for a missing key, not an exception. The caller has to remember.

What's wrong with this code?

stockByProduct.get("headphones") returns null because the key isn't there. Assigning null to an int requires unboxing, which fails on null. The crash happens on the assignment line, not on the missing-key line.

Fix:

getOrDefault returns the supplied default when the key is missing. If 0 isn't the right answer, use containsKey first or wrap the lookup in Optional.ofNullable.

Uninitialized Instance Fields

Java initializes instance fields to default values (null for references, 0 for numbers). The default for an object reference is null, which surprises people who expect "empty."

What's wrong with this code?

items was declared but never assigned, so it defaults to null. Calling .add on null throws.

Fix:

Initialize collection fields at declaration or in the constructor. Treat an empty list as the right "nothing in the wishlist yet" representation, never null.

== compares references. equals compares contents. For strings, almost always use equals.

What's wrong with this code?

"ADMIN10" (the literal) lives in the string pool. new String("ADMIN10") creates a fresh object outside the pool. They hold the same characters but they're different objects, so == is false.

Fix:

equals checks character-by-character. For null safety, flip the order so the literal is on the left ("ADMIN10".equals(entered)) or use Objects.equals(entered, coupon).

Integer values from -128 to 127 are cached. The same number outside that range produces a new object each time, and == returns false.

The autoboxing of 100 returns the same cached Integer. The autoboxing of 200 allocates two different objects. == distinguishes them; equals doesn't. The cache range is a JVM optimization, not part of your program's logic.

The rule is simple. For wrapper types like Integer, Long, Double, always use equals. Save == for primitives or for genuine reference identity checks.

int operations wrap silently when they exceed Integer.MAX_VALUE. No exception. No warning. Just a wrong answer.

What's wrong with this code?

50_000 * 100_000 is five billion, way past Integer.MAX_VALUE (about 2.1 billion). The multiplication overflows and wraps, giving a nonsense total. The customer is charged either too much or too little, and nothing in the program complains.

Fix:

There are two fixes. Use long when the result might exceed int range. Or use Math.multiplyExact, Math.addExact, Math.subtractExact for explicit overflow detection. They throw ArithmeticException instead of wrapping. For prices, use BigDecimal (see below) and skip the problem entirely.

Math.multiplyExact adds an overflow check on every call. The cost is small but real. Use long first; use Math.*Exact when you specifically need to detect overflow at the call site.

Binary floating point can't represent most decimals exactly. 0.1 + 0.2 is not 0.3. For money, this is a bug.

What's wrong with this code?

double stores numbers in base-2 fractions. 0.1 and 0.2 are infinite repeating sequences in base-2, so the stored values are approximations. The sum carries a tiny rounding error. For a cart total in cents this matters; for a customer review score it usually doesn't.

Fix:

BigDecimal stores exact decimal values. Construct it from a String, not a double. Calling new BigDecimal(0.1) produces the same lossy approximation that double already had. And use compareTo (returns 0 for equal) rather than equals, because equals also compares the scale ("0.30" does not equal "0.3").

Autoboxing converts a primitive to its wrapper (int to Integer) silently. In a tight loop, each conversion allocates an object.

What's wrong with this code?

The output is correct, but the cost is hidden. total is Long (the wrapper). Every total += i unboxes total to long, adds, and boxes the result back to a new Long. A million iterations means a million unnecessary Long allocations. The fix is to declare total as long.

Fix:

Same answer, none of the allocations. The other autobox hazard is the NPE one: a Long field that's null throws a NullPointerException the moment you unbox it. Both reasons argue for the same rule. Use primitive types unless you specifically need null or you're putting values into a collection.

A static field is shared across every instance of the class. Make it mutable and you've created a global variable in a language that pretends not to have them.

What's wrong with this code?

items is static, so both carts share the same list. Rohan's headphones show up in Aanya's cart, and vice versa. The bug is rare to spot in single-threaded testing because the test usually constructs one object at a time.

Fix:

Remove static. Each cart now has its own list. Reserve static for true constants (static final int MAX_ITEMS = 99) or for stateless helper methods. Mutable static state is a bug factory, especially once threads are involved.

Modifying a collection while iterating over it (in the same thread, no concurrency required) throws ConcurrentModificationException.

What's wrong with this code?

The enhanced-for loop uses an iterator internally. The iterator records the list's modification count when it starts. Each call to cart.remove bumps the modification count, and the iterator's next call to next() notices the mismatch and throws. The name is misleading; the "concurrent" doesn't require multiple threads.

Fix:

There are two fixes. Call it.remove() on the iterator, which keeps the modification count in sync. Or use Collection.removeIf with a predicate, which is shorter and avoids the iterator boilerplate. Both produce the same result. For single-threaded code, these two patterns cover the cases.

HashMap and HashSet rely on a contract: if two objects are equal per equals, they must produce the same hashCode. Mutating an object after putting it in a hash-based collection breaks the contract by changing the hash code while the bucket assignment stays the same.

What's wrong with this code?

The map puts notebook in the bucket for its original hash code. Mutating categoryId changes the hash code, so stock.get(notebook) looks in a different bucket. The value is in the map but unreachable. The map size is still 1, but no key in it can be found.

Fix:

Make the fields used in equals and hashCode final. The class becomes effectively immutable in the parts that matter for hashing. To "change" the category, build a new Product instead. The same rule applies to record classes (they're immutable by default) and to String (immutable, which is exactly why it's a safe hash key).

A single bad mutation can render any number of entries unreachable in a HashMap. The map's memory is still occupied, but get returns null and containsKey returns false. The bug is silent and very hard to spot without targeted tests.

Resources that hold OS handles (files, sockets, database connections) must be closed. A pre-try-with-resources style is easy to get wrong.

What's wrong with this code?

The bug isn't visible in the output. If the write throws, close never runs and the file handle leaks. If the return fires, the same problem. Even adding try/finally is verbose and easy to mess up (a null writer, a close-time exception masking the original).

Fix:

try-with-resources calls close automatically when the block exits, whether by return, exception, or normal flow. If the close itself throws, its exception is attached as suppressed on the original, so neither is lost. The rule for this lesson is: any AutoCloseable belongs in a try-with-resources block.

A broad catch silences every failure in its scope. The right exception to catch is the narrowest one that fits.

What's wrong with this code?

The catch hides what actually failed. A NumberFormatException from a bad input string and a NullPointerException from a typo three lines up get the same handler. Monitoring sees one log line for both cases. The user sees an unhelpful message. Future bugs will hide behind the same blanket.

Fix:

Catch NumberFormatException directly. The message names the bad input. Any other exception type propagates, where it can be handled at the right layer or surfaced as the bug it is.

Strings are immutable. Each + allocates a new String. In a loop, that turns into thousands of allocations.

What's wrong with this code?

The output is right, but every iteration builds a brand-new String containing all previous characters. For five items it's fine. For five thousand items it allocates roughly 12.5 million characters of garbage. The compiler does optimize a single a + b + c expression into a StringBuilder, but it can't optimize the loop because each iteration's input depends on the previous result.

Fix:

StringBuilder keeps a mutable buffer that grows as needed. One allocation up front, occasional reallocations as the buffer doubles, one final toString at the end. For comma-separated joins, String.join(", ", items) is shorter still. Reserve + for short expressions where readability wins over the few cycles you'd save.

A naive concatenation loop is roughly O(n^2) in the total characters produced. The same job with StringBuilder is O(n). The difference is invisible at small sizes and severe at large ones.

A method that returns null for "no results" forces every caller to write a null check before iterating. Forget once and you get an NPE.

What's wrong with this code?

The caller can't tell from the signature that null is possible. The first time the unknown customer path fires, the NPE shows up.

Fix:

Return Collections.emptyList(), List.of(), or Set.of() for the no-results case. Iteration is a no-op. .size() returns 0. The caller writes one path that handles both cases. When the absence carries meaning (the customer doesn't exist at all, versus the customer exists but has no wishlist), use Optional<List<String>> to encode the distinction in the type.

Two double values that "should" be equal often aren't, because of rounding. Compare with a tolerance instead.

What's wrong with this code?

Adding 0.1 ten times doesn't land exactly at 1.0. The accumulated rounding error leaves you a hair short. The == check fails.

Fix:

Compare the absolute difference to a small tolerance. The right tolerance depends on the values involved (1e-9 for unit-scale numbers, larger for accumulated sums of bigger numbers). For money, use BigDecimal and don't have the problem in the first place.

Arrays.asList returns a fixed-size list backed by the original array. You can replace elements but you can't change its size.

What's wrong with this code?

Arrays.asList does not return an ArrayList. It returns a thin wrapper around the array, with a fixed size. add and remove throw UnsupportedOperationException. The same gotcha applies to List.of(...), which returns an immutable list where even set throws.

Fix:

Wrap the fixed-size list in a new ArrayList. The copy is independent of the array, and resizing operations work. The rule: if you need to add or remove, construct a mutable list explicitly.

Classic switch statements fall through from one case to the next when there's no break. Almost never what you want.

What's wrong with this code?

The matching case (SHIPPED) runs. Then DELIVERED runs. Then default runs. Without break between cases, execution falls through to the next. The output is three lines instead of one. The bug is silent in compilers without exhaustive warnings.

Fix:

The arrow form (->, introduced as a preview in Java 12 and standardized in Java 14) doesn't fall through. Each case is independent. If you're stuck on classic switch, add a break to every case. Modern Java code should use the arrow form unless fall-through is the intent (and then it deserves a comment).

A constructor that calls an overridable method may run the subclass's override before the subclass's own fields are initialized.

What's wrong with this code?

Construction order: the Product constructor runs first. It calls describe, which dispatches to DiscountedProduct.describe (virtual dispatch always picks the subclass override). But DiscountedProduct's field initializers haven't run yet, so discount is still 0.0. The print line lies.

Fix:

There are two changes. The constructor calls a private helper, which can't be overridden. The describe method is called after construction completes. The rule: constructors should only call methods that are private, static, or final. Anything overridable is error-prone, waiting for a subclass.

The shortest summary of the pitfalls above. Use it as a checklist on your next code review.

The fix column is the rule of thumb, not a substitute for understanding what each pitfall actually causes. Read the bug, read the explanation, and the fix follows from the why.