Code Review · Java

Java Code Review: NullPointerException Risk

Review Java code that assumes inputs and object fields are never null.

Code ReviewJavaNull SafetyBugs

Code Review Exercise

Review the following Java code. The method looks simple, but it has several reliability and maintainability issues.

java

public class OrderService {
    public boolean shouldApplyDiscount(Order order) {
        return order.getCustomer().getStatus().equals("VIP")
            && order.getTotalAmount() > 100;
    }
}

What issues do you see before scrolling further?

Review Findings

Issue #1: Chained calls can throw NullPointerException

Any part of this chain can be null: order, getCustomer(), or getStatus(). If one of them is null, the method throws instead of returning a safe answer.'

Issue #2: equals() is called on a possibly null value

order.getCustomer().getStatus().equals("VIP") will fail if getStatus() returns null. A constant-first comparison is safer: "VIP".equals(status).

Issue #3: Magic string

"VIP" is a business value hidden as a raw string. If customer status is important to the domain, an enum or constant would make the code safer and easier to refactor.

Issue #4: Business logic is hard to read

The method mixes null navigation, status comparison, and amount logic in one expression. Breaking it into named steps makes the intent easier to review.

Why This Is Easy To Miss

The code reads nicely from left to right, which makes it feel clean. But chained method calls hide risk. In code review, a good habit is to ask: which values are guaranteed to be non-null?

A clean-looking one-liner can still be fragile if it assumes too much about the data.

Fixed Version: Guard Clauses

One simple fix is to validate each required object before using it. This makes the null behavior explicit.

java

public class OrderService {
    private static final String VIP_STATUS = "VIP";

    public boolean shouldApplyDiscount(Order order) {
        if (order == null) return false;
        if (order.getCustomer() == null) return false;

        String status = order.getCustomer().getStatus();

        return VIP_STATUS.equals(status)
            && order.getTotalAmount() > 100;
    }
}

This version is not fancy, but it is clear. In interviews, clear and correct is usually better than clever.

Better Domain Model: Use an Enum

If customer status is a known set of values, an enum is usually better than strings.

java

enum CustomerStatus {
    REGULAR,
    VIP,
    BLOCKED
}

public class OrderService {
    public boolean shouldApplyDiscount(Order order) {
        if (order == null || order.getCustomer() == null) {
            return false;
        }

        Customer customer = order.getCustomer();

        return CustomerStatus.VIP == customer.getStatus()
            && order.getTotalAmount() > 100;
    }
}

Using == with enums is normal in Java because enum constants are single instances. This also avoids typo bugs like "VPI".

Should You Use Optional Here?

Optional can be useful as a return type, but it should not be used just to hide unclear object ownership or messy domain modeling.

In a code review, a strong answer is not simply: “use Optional.” A stronger answer is: “first clarify which fields are allowed to be null, then make that contract obvious in the code.”

What Interviewers Expect

Catch the immediate correctness issue: possible NullPointerException.
Mention safer equality: "VIP".equals(status).
Suggest improving the domain model with an enum or constant.
Explain the tradeoff: guard clauses are simple, explicit, and easy to review.

Interview Follow-Ups

Is every NullPointerException fixed by adding more null checks?

No. Sometimes the better fix is to enforce invariants earlier, validate input at the boundary, or improve the domain model so impossible states cannot be represented.

Why is constant-first equals safer?

Because the constant is never null. "VIP".equals(status) returns false when status is null, while status.equals("VIP") throws NullPointerException.

When is == okay in Java?

It is okay for primitives and enums. For most objects such as String, use equals() for logical equality.

What is the maintainability issue with long chained calls?

They hide assumptions. A future reader has to know whether every object in the chain is guaranteed to exist. Named local variables and guard clauses often make the code easier to understand.