Common Java Pitfalls¶

// ❌ Caller has no indication the result may be absent
public Order findById(String id) {
    return repository.find(id);  // may return null
}

// ✅ Explicit absence signalled by return type
public Optional<Order> findById(String id) {
    return Optional.ofNullable(repository.find(id));
}

// ❌ Never chain without a null check
order.getCustomer().getAddress().getCity()

// ✅ Safe navigation
String city = Optional.ofNullable(order)
    .map(Order::getCustomer)
    .map(Customer::getAddress)
    .map(Address::getCity)
    .orElse("Unknown");

// ❌ Catches OutOfMemoryError, interrupted signals, and everything else
try {
    processOrder(order);
} catch (Exception e) {
    log.error("Error");  // And swallowed — no rethrow!
}

// ✅ Catch the specific type you can handle
try {
    processOrder(order);
} catch (OrderValidationException e) {
    log.warn("Invalid order: {}", e.getMessage());
    throw new BadRequestException(e.getMessage(), e);
}

// ❌ Shared mutable state
private static List<Order> pendingOrders = new ArrayList<>();

// ✅ Inject state through the constructor
public class OrderProcessor {
    private final OrderRepository repository;

    public OrderProcessor(OrderRepository repository) {
        this.repository = repository;
    }
}

// ✅ Use record for value objects — equals/hashCode/toString are generated
record OrderId(String value) {}

// If you must use a class — always override both
@Override
public boolean equals(Object o) { ... }

@Override
public int hashCode() { return Objects.hash(id); }

// ❌ Creates a new String on each iteration
String result = "";
for (String part : parts) {
    result += part;
}

// ✅ Use StringBuilder
StringBuilder sb = new StringBuilder();
for (String part : parts) {
    sb.append(part);
}
String result = sb.toString();

// ✅ Or use String.join / Collectors.joining for simple cases
String result = String.join(", ", parts);

// ❌ Throws ConcurrentModificationException
for (Order order : orders) {
    if (order.isExpired()) {
        orders.remove(order);
    }
}

// ✅ Use removeIf
orders.removeIf(Order::isExpired);

// ✅ Or collect to a new list
List<Order> active = orders.stream()
    .filter(Predicate.not(Order::isExpired))
    .toList();