Java Best Practices

// BAD - unclear what these represent
boolean flag = true;
boolean status = false;
boolean check = user.verify();

// GOOD - reads like a question: "Is it active?" -> true/false
boolean isActive = true;
boolean hasPermission = false;
boolean canExecute = user.hasRole("ADMIN");
boolean shouldRetry = attempts < MAX_RETRIES;
boolean wasProcessed = order.getStatus() == Status.COMPLETE;

// BAD - abbreviations force the reader to decode
int qty;
String addr;
UserAcctMgr uam;
public void calcTtlAmt() { ... }

// GOOD - instantly clear
int quantity;
String address;
UserAccountManager userAccountManager;
public void calculateTotalAmount() { ... }

// BAD - what does d represent? What are the 86400000?
long d = System.currentTimeMillis() - start;
if (d > 86400000) { ... }

// GOOD - the name tells the whole story
long elapsedTimeMillis = System.currentTimeMillis() - startTimeMillis;
long ONE_DAY_IN_MILLIS = 24 * 60 * 60 * 1000L;
if (elapsedTimeMillis > ONE_DAY_IN_MILLIS) { ... }

// BAD - missing braces, inconsistent spacing, dense
if(active) doSomething();
for(int i=0;i<10;i++) process(i);
if (x > 0)
    doThis();
    doThat(); // BUG: doThat() runs ALWAYS, not inside the if

// GOOD - consistent braces, spacing, and structure
if (active) {
    doSomething();
}

for (int i = 0; i < 10; i++) {
    process(i);
}

if (x > 0) {
    doThis();
    doThat(); // clearly inside the if block
}

// BAD - comments that repeat the code (add nothing)
// Increment counter by 1
counter++;

// Check if user is null
if (user == null) {
    return;
}

// Get the list of users from the database
List users = userRepository.findAll();

// GOOD - comments that explain WHY
// Circuit breaker: stop retrying after 3 failures to avoid
// cascading timeouts to downstream services
if (failureCount >= MAX_FAILURES) {
    return fallbackResponse();
}

// Using insertion order map because the API contract guarantees
// fields are returned in the order they were added
Map fields = new LinkedHashMap<>();

/**
 * Transfers funds between two accounts within a single transaction.
 *
 * 
Both accounts must be active and belong to the same currency.
 * The transfer is atomic -- if any step fails, both accounts
 * remain unchanged.
 *
 * @param fromAccountId the source account ID
 * @param toAccountId   the destination account ID
 * @param amount        the amount to transfer (must be positive)
 * @return the transaction confirmation with a unique reference ID
 * @throws InsufficientFundsException if the source account balance is too low
 * @throws AccountNotFoundException   if either account does not exist
 * @throws IllegalArgumentException   if amount is zero or negative
 */
public TransactionConfirmation transfer(
        String fromAccountId,
        String toAccountId,
        BigDecimal amount) {
    // implementation
}

// BAD - Single Responsibility violation: one class does everything
public class UserService {
    public void registerUser(User user) { ... }
    public void sendWelcomeEmail(User user) { ... }   // email logic
    public void generatePdfReport(User user) { ... }  // PDF logic
    public boolean validateUser(User user) { ... }     // validation logic
}

// GOOD - each class has one responsibility
public class UserService {
    private final UserValidator validator;
    private final UserRepository repository;
    private final NotificationService notificationService;

    public UserService(UserValidator validator,
                       UserRepository repository,
                       NotificationService notificationService) {
        this.validator = validator;
        this.repository = repository;
        this.notificationService = notificationService;
    }

    public void registerUser(User user) {
        validator.validate(user);
        repository.save(user);
        notificationService.sendWelcomeNotification(user);
    }
}

// BAD - inheritance for code reuse (fragile base class problem)
public class Logger {
    public void log(String message) {
        System.out.println("[LOG] " + message);
    }
}

public class UserService extends Logger {
    // UserService IS-A Logger? That makes no sense.
    public void createUser(String name) {
        log("Creating user: " + name);  // inherited method
        // ...
    }
}

// GOOD - composition: UserService HAS-A Logger
public class UserService {
    private final Logger logger;

    public UserService(Logger logger) {
        this.logger = logger;
    }

    public void createUser(String name) {
        logger.log("Creating user: " + name);
        // ...
    }
}
// Now you can swap FileLogger, ConsoleLogger, or NoOpLogger
// without touching UserService at all.

// BAD - tied to a specific implementation
ArrayList names = new ArrayList<>();
HashMap userMap = new HashMap<>();

public ArrayList getNames() {
    return names;
}

// GOOD - programmed to the interface
List names = new ArrayList<>();
Map userMap = new HashMap<>();

public List getNames() {
    return names;
}
// Tomorrow you can switch to LinkedList or CopyOnWriteArrayList
// without changing any calling code.

// BAD - hard dependency on a concrete class
public class OrderService {
    private MySQLOrderRepository repository = new MySQLOrderRepository();
}

// GOOD - depends on an abstraction, injected via constructor
public class OrderService {
    private final OrderRepository repository;

    public OrderService(OrderRepository repository) {
        this.repository = repository;
    }
}
// In production: new OrderService(new MySQLOrderRepository())
// In tests:      new OrderService(new FakeOrderRepository())

// BAD - mutable class with exposed internals
public class Order {
    public List items;  // anyone can modify this list
    public double total;        // anyone can change the total

    public Order(List items) {
        this.items = items;  // stores a reference -- caller can mutate it
    }
}

// GOOD - immutable class with defensive copying
public final class Order {
    private final List items;
    private final double total;

    public Order(List items, double total) {
        // Defensive copy: changes to the original list won't affect us
        this.items = List.copyOf(items);
        this.total = total;
    }

    public List getItems() {
        // Returns an unmodifiable view -- caller cannot add/remove items
        return items; // already unmodifiable from List.copyOf()
    }

    public double getTotal() {
        return total;
    }
}

// Java 9+ factory methods create unmodifiable collections
List colors = List.of("Red", "Green", "Blue");
Set primes = Set.of(2, 3, 5, 7, 11);
Map scores = Map.of("Alice", 95, "Bob", 87);

// For mutable lists you need to return unmodifiable views
public List getUsers() {
    return Collections.unmodifiableList(users);
}

// Java 16+ records are inherently immutable (no setter generation)
public record Money(BigDecimal amount, Currency currency) { }

// BAD - catches everything, including programming errors
try {
    processOrder(order);
} catch (Exception e) {
    log.error("Something went wrong");  // what went wrong? No clue.
}

// GOOD - catch specific exceptions, handle each appropriately
try {
    processOrder(order);
} catch (InsufficientFundsException e) {
    log.warn("Insufficient funds for order {}: {}", order.getId(), e.getMessage());
    notifyCustomer(order, "Payment failed: insufficient funds");
} catch (InventoryException e) {
    log.error("Inventory check failed for order {}", order.getId(), e);
    queueForRetry(order);
}

// BAD - the worst thing you can do with an exception
try {
    Files.delete(tempFile);
} catch (IOException e) {
    // silently ignored -- if the delete fails, you'll never know
}

// GOOD - at minimum, log it. Better: handle it.
try {
    Files.delete(tempFile);
} catch (IOException e) {
    log.warn("Failed to delete temp file {}: {}", tempFile, e.getMessage());
}

// BAD - manual resource management (verbose and error-prone)
Connection conn = null;
PreparedStatement stmt = null;
try {
    conn = dataSource.getConnection();
    stmt = conn.prepareStatement("SELECT * FROM users WHERE id = ?");
    stmt.setLong(1, userId);
    ResultSet rs = stmt.executeQuery();
    // process results
} catch (SQLException e) {
    log.error("Query failed", e);
} finally {
    if (stmt != null) { try { stmt.close(); } catch (SQLException e) { } }
    if (conn != null) { try { conn.close(); } catch (SQLException e) { } }
}

// GOOD - try-with-resources handles all cleanup automatically
try (Connection conn = dataSource.getConnection();
     PreparedStatement stmt = conn.prepareStatement(
         "SELECT * FROM users WHERE id = ?")) {

    stmt.setLong(1, userId);
    try (ResultSet rs = stmt.executeQuery()) {
        // process results
    }
} catch (SQLException e) {
    log.error("Query failed for user {}", userId, e);
}

public class InsufficientFundsException extends RuntimeException {
    private final BigDecimal currentBalance;
    private final BigDecimal requestedAmount;

    public InsufficientFundsException(BigDecimal currentBalance,
                                       BigDecimal requestedAmount) {
        super(String.format("Cannot withdraw %s: current balance is %s",
              requestedAmount, currentBalance));
        this.currentBalance = currentBalance;
        this.requestedAmount = requestedAmount;
    }

    public BigDecimal getCurrentBalance() { return currentBalance; }
    public BigDecimal getRequestedAmount() { return requestedAmount; }
}

// Usage: fail fast with a meaningful exception
public void withdraw(BigDecimal amount) {
    if (amount.compareTo(balance) > 0) {
        throw new InsufficientFundsException(balance, amount);
    }
    balance = balance.subtract(amount);
}

// BAD - forces every caller to check for null
public List getOrdersByCustomer(String customerId) {
    List orders = repository.findByCustomerId(customerId);
    if (orders.isEmpty()) {
        return null;  // caller MUST check: if (orders != null)
    }
    return orders;
}

// GOOD - return empty collection, callers can iterate safely
public List getOrdersByCustomer(String customerId) {
    List orders = repository.findByCustomerId(customerId);
    return orders != null ? orders : Collections.emptyList();
}

// Even better with modern Java:
public List getOrdersByCustomer(String customerId) {
    return Objects.requireNonNullElse(
        repository.findByCustomerId(customerId),
        List.of()
    );
}

// BAD - returning null for "not found"
public User findByEmail(String email) {
    // Returns null if not found -- caller might forget to check
    return userMap.get(email);
}

// GOOD - Optional makes the absence explicit
public Optional findByEmail(String email) {
    return Optional.ofNullable(userMap.get(email));
}

// Callers are forced to handle both cases:
userService.findByEmail("alice@example.com")
    .ifPresentOrElse(
        user -> System.out.println("Found: " + user.getName()),
        () -> System.out.println("User not found")
    );

// Or with a default:
User user = userService.findByEmail("alice@example.com")
    .orElseThrow(() -> new UserNotFoundException("alice@example.com"));

public class EmailService {
    private final SmtpClient client;
    private final TemplateEngine templateEngine;

    public EmailService(SmtpClient client, TemplateEngine templateEngine) {
        // Fail immediately with a clear message instead of getting a
        // NullPointerException 5 method calls later
        this.client = Objects.requireNonNull(client, "SmtpClient must not be null");
        this.templateEngine = Objects.requireNonNull(templateEngine,
            "TemplateEngine must not be null");
    }
}

// BAD - creates a new String object on every iteration
// For 10,000 iterations, this creates ~10,000 intermediate String objects
public String buildReport(List lines) {
    String result = "";
    for (String line : lines) {
        result += line + "\n";  // new String every time
    }
    return result;
}

// GOOD - StringBuilder reuses a single internal buffer
public String buildReport(List lines) {
    StringBuilder sb = new StringBuilder(lines.size() * 80); // pre-size estimate
    for (String line : lines) {
        sb.append(line).append("\n");
    }
    return sb.toString();
}

// ALSO GOOD - String.join() for simple cases
public String buildReport(List lines) {
    return String.join("\n", lines);
}

// BAD - unreadable concatenation of a multi-line string
String json = "{\n" +
    "  \"name\": \"" + name + "\",\n" +
    "  \"email\": \"" + email + "\"\n" +
    "}";

// GOOD - text block (Java 13+) with formatted() (Java 15+)
String json = """
    {
      "name": "%s",
      "email": "%s"
    }
    """.formatted(name, email);

// BAD - wrong collection for the job
LinkedList users = new LinkedList<>();  // random access is O(n)
users.get(500);  // traverses 500 nodes to get element

// GOOD - ArrayList for indexed access
ArrayList users = new ArrayList<>();
users.get(500);  // O(1) array index lookup

// Pre-size when you know the approximate size (avoids resizing)
List names = new ArrayList<>(1000);
Map cache = new HashMap<>(256);

// Use Collections utility methods for common operations
List numbers = Arrays.asList(5, 3, 8, 1, 9);
Collections.sort(numbers);                    // sort in place
int max = Collections.max(numbers);           // find max
Collections.shuffle(numbers);                 // random order
List synced = Collections.synchronizedList(numbers);

// OVERKILL - stream adds nothing for a simple loop
users.stream().forEach(user -> user.sendEmail());

// BETTER - plain for-each is simpler
for (User user : users) {
    user.sendEmail();
}

// GOOD USE OF STREAM - transformation, filtering, collecting
List activeEmails = users.stream()
    .filter(User::isActive)
    .map(User::getEmail)
    .sorted()
    .collect(Collectors.toList());

// Read a file, process each line, write results to another file
public void processFile(Path input, Path output) throws IOException {
    try (BufferedReader reader = Files.newBufferedReader(input);
         BufferedWriter writer = Files.newBufferedWriter(output)) {

        String line;
        while ((line = reader.readLine()) != null) {
            String processed = transform(line);
            writer.write(processed);
            writer.newLine();
        }
    }
    // Both reader and writer are automatically closed here,
    // even if an exception occurred
}

// For custom resources, implement AutoCloseable
public class ConnectionPool implements AutoCloseable {
    private final List connections = new ArrayList<>();

    public Connection acquire() { /* ... */ }

    @Override
    public void close() {
        connections.forEach(conn -> {
            try { conn.close(); } catch (SQLException e) {
                log.warn("Failed to close connection", e);
            }
        });
        connections.clear();
    }
}

// GOOD uses of var - type is obvious from context
var names = new ArrayList();         // clearly ArrayList
var reader = new BufferedReader(new FileReader("data.txt"));
var count = 0;                               // clearly int
var entry = map.entrySet().iterator().next(); // avoids verbose generic type

// BAD uses of var - type is unclear
var result = process();    // what type is result?
var data = getData();      // what type is data?
var x = calculate(a, b);  // impossible to understand without IDE

// BAD - 50 lines of boilerplate for a simple data class
public class Point {
    private final int x;
    private final int y;

    public Point(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() { return x; }
    public int getY() { return y; }

    @Override
    public boolean equals(Object o) { /* 10 lines */ }
    @Override
    public int hashCode() { return Objects.hash(x, y); }
    @Override
    public String toString() { return "Point[x=" + x + ", y=" + y + "]"; }
}

// GOOD - one line with Java 16+ records
public record Point(int x, int y) { }

// Records can have validation and custom methods
public record EmailAddress(String value) {
    public EmailAddress {
        // Compact constructor for validation
        if (value == null || !value.contains("@")) {
            throw new IllegalArgumentException("Invalid email: " + value);
        }
    }
}

// Sealed class - only these three can extend Shape
public sealed interface Shape
    permits Circle, Rectangle, Triangle {
}

public record Circle(double radius) implements Shape { }
public record Rectangle(double width, double height) implements Shape { }
public record Triangle(double base, double height) implements Shape { }

// The compiler KNOWS all subtypes, so switch can be exhaustive (Java 21+)
public double area(Shape shape) {
    return switch (shape) {
        case Circle c    -> Math.PI * c.radius() * c.radius();
        case Rectangle r -> r.width() * r.height();
        case Triangle t  -> 0.5 * t.base() * t.height();
        // No default needed -- compiler verifies all cases are covered
    };
}

// BAD - traditional switch with fall-through risk
String dayType;
switch (day) {
    case MONDAY:
    case TUESDAY:
    case WEDNESDAY:
    case THURSDAY:
    case FRIDAY:
        dayType = "Weekday";
        break;     // forget this break and you get a bug
    case SATURDAY:
    case SUNDAY:
        dayType = "Weekend";
        break;
    default:
        dayType = "Unknown";
}

// GOOD - switch expression (Java 14+), no fall-through possible
String dayType = switch (day) {
    case MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY -> "Weekday";
    case SATURDAY, SUNDAY -> "Weekend";
};

// BAD - old way: check instanceof, then cast
if (obj instanceof String) {
    String s = (String) obj;  // redundant cast
    System.out.println(s.length());
}

// GOOD - pattern matching (Java 16+): check and cast in one step
if (obj instanceof String s) {
    System.out.println(s.length());
}

// Works great with negation too
if (!(obj instanceof String s)) {
    return; // early exit
}
// s is in scope here
System.out.println(s.toUpperCase());

// BAD - overly complex stream, hard to read and debug
Map> result = orders.stream()
    .filter(o -> o.getStatus() != null && o.getStatus().equals("ACTIVE")
        && o.getTotal() != null && o.getTotal().compareTo(BigDecimal.ZERO) > 0)
    .sorted(Comparator.comparing(Order::getCreatedAt).reversed())
    .map(o -> new OrderDTO(o.getId(), o.getCustomer().getName(),
        o.getItems().stream().map(Item::getName).collect(Collectors.joining(", ")),
        o.getTotal()))
    .collect(Collectors.groupingBy(OrderDTO::customerName));

// GOOD - break it into readable steps
List activeOrders = orders.stream()
    .filter(this::isActiveWithPositiveTotal)
    .sorted(Comparator.comparing(Order::getCreatedAt).reversed())
    .toList();

Map> result = activeOrders.stream()
    .map(this::toOrderDTO)
    .collect(Collectors.groupingBy(OrderDTO::customerName));

// Helper methods make it self-documenting
private boolean isActiveWithPositiveTotal(Order order) {
    return "ACTIVE".equals(order.getStatus())
        && order.getTotal() != null
        && order.getTotal().compareTo(BigDecimal.ZERO) > 0;
}

private OrderDTO toOrderDTO(Order order) {
    String itemNames = order.getItems().stream()
        .map(Item::getName)
        .collect(Collectors.joining(", "));
    return new OrderDTO(order.getId(),
        order.getCustomer().getName(), itemNames, order.getTotal());
}

// OK - lambda
names.stream().map(name -> name.toUpperCase()).collect(Collectors.toList());
names.stream().filter(name -> name.isEmpty()).count();

// BETTER - method reference (cleaner, less noise)
names.stream().map(String::toUpperCase).collect(Collectors.toList());
names.stream().filter(String::isEmpty).count();

// More method reference examples
users.stream().map(User::getEmail)...                  // instance method
numbers.stream().forEach(System.out::println);          // static method
names.stream().map(String::new)...                      // constructor

// BAD - side effects: modifying external list inside a stream
List results = new ArrayList<>();
names.stream()
    .filter(n -> n.length() > 3)
    .forEach(n -> results.add(n));  // mutating external state

// GOOD - collect into a new list (no side effects)
List results = names.stream()
    .filter(n -> n.length() > 3)
    .collect(Collectors.toList());

// Or with Java 16+
List results = names.stream()
    .filter(n -> n.length() > 3)
    .toList();

// BAD - untestable: creates its own dependencies
public class OrderProcessor {
    private final PaymentGateway gateway = new StripePaymentGateway();
    private final EmailService email = new SmtpEmailService();

    public void process(Order order) {
        gateway.charge(order.getTotal());  // hits real Stripe API in tests!
        email.send(order.getEmail(), "Order confirmed");  // sends real emails!
    }
}

// GOOD - testable: dependencies are injected
public class OrderProcessor {
    private final PaymentGateway gateway;
    private final EmailService email;

    public OrderProcessor(PaymentGateway gateway, EmailService email) {
        this.gateway = gateway;
        this.email = email;
    }

    public void process(Order order) {
        gateway.charge(order.getTotal());
        email.send(order.getEmail(), "Order confirmed");
    }
}

// In tests, inject fakes or mocks:
@Test
void shouldChargeAndSendEmail() {
    var fakeGateway = new FakePaymentGateway();
    var fakeEmail = new FakeEmailService();
    var processor = new OrderProcessor(fakeGateway, fakeEmail);

    processor.process(testOrder);

    assertTrue(fakeGateway.wasCharged());
    assertEquals("Order confirmed", fakeEmail.getLastMessage());
}

// BAD - test names that tell you nothing when they fail
@Test void test1() { ... }
@Test void testProcess() { ... }
@Test void testUser() { ... }

// GOOD - test names describe scenario and expected behavior
@Test void withdraw_withSufficientFunds_shouldReduceBalance() { ... }
@Test void withdraw_withInsufficientFunds_shouldThrowException() { ... }
@Test void withdraw_withNegativeAmount_shouldThrowIllegalArgument() { ... }
@Test void findByEmail_withNonexistentEmail_shouldReturnEmpty() { ... }

// ALSO GOOD - Given/When/Then style
@Test void givenInactiveUser_whenLogin_thenAccountLockedExceptionThrown() { ... }

// BAD - static utility for business logic is untestable in callers
public class PricingUtils {
    public static BigDecimal calculateDiscount(Order order) {
        // Complex business logic tied to a static method
        // Callers cannot replace this in tests
    }
}

// GOOD - instance method on an injectable service
public class PricingService {
    public BigDecimal calculateDiscount(Order order) {
        // Same logic, but now callers can inject a fake
    }
}

// Static methods are fine for pure utility functions with no side effects:
public final class StringUtils {
    public static boolean isBlank(String s) {
        return s == null || s.trim().isEmpty();
    }
}

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class PaymentService {
    // One logger per class, declared as a static final field
    private static final Logger log = LoggerFactory.getLogger(PaymentService.class);

    public PaymentResult processPayment(PaymentRequest request) {
        log.info("Processing payment for order {}", request.getOrderId());

        try {
            PaymentResult result = gateway.charge(request);
            log.info("Payment successful: orderId={}, transactionId={}",
                request.getOrderId(), result.getTransactionId());
            return result;
        } catch (PaymentDeclinedException e) {
            log.warn("Payment declined for order {}: {}",
                request.getOrderId(), e.getReason());
            throw e;
        } catch (Exception e) {
            log.error("Payment processing failed for order {}",
                request.getOrderId(), e);  // exception as last arg logs stack trace
            throw new PaymentProcessingException("Failed to process payment", e);
        }
    }
}

// BAD - string concatenation happens even if DEBUG is disabled
log.debug("Processing user " + user.getName() + " with id " + user.getId());

// GOOD - parameterized: {} placeholders are only resolved if level is enabled
log.debug("Processing user {} with id {}", user.getName(), user.getId());

// BAD - checking log level AND using concatenation (defeats the purpose)
if (log.isDebugEnabled()) {
    log.debug("Processing user " + user.getName());
}

// GOOD - parameterized logging handles the check internally
log.debug("Processing user {}", user.getName());

// BAD - package by layer (a change to "orders" touches 4 packages)
com.example.app
├── controller/
│   ├── OrderController.java
│   ├── UserController.java
│   └── ProductController.java
├── service/
│   ├── OrderService.java
│   ├── UserService.java
│   └── ProductService.java
├── repository/
│   ├── OrderRepository.java
│   ├── UserRepository.java
│   └── ProductRepository.java
└── model/
    ├── Order.java
    ├── User.java
    └── Product.java

// GOOD - package by feature (everything about "orders" is together)
com.example.app
├── order/
│   ├── OrderController.java
│   ├── OrderService.java
│   ├── OrderRepository.java
│   ├── Order.java
│   └── OrderDTO.java
├── user/
│   ├── UserController.java
│   ├── UserService.java
│   ├── UserRepository.java
│   └── User.java
└── product/
    ├── ProductController.java
    ├── ProductService.java
    ├── ProductRepository.java
    └── Product.java

// BAD - constructor with many parameters (which is email? which is phone?)
User user = new User("Alice", "Smith", "alice@example.com",
    "555-1234", "123 Main St", "Springfield", "IL", "62701", true, false);

// GOOD - Builder pattern: every parameter is labeled
User user = User.builder()
    .firstName("Alice")
    .lastName("Smith")
    .email("alice@example.com")
    .phone("555-1234")
    .address(Address.builder()
        .street("123 Main St")
        .city("Springfield")
        .state("IL")
        .zipCode("62701")
        .build())
    .isActive(true)
    .isAdmin(false)
    .build();

public class User {
    private final String firstName;
    private final String lastName;
    private final String email;

    private User(Builder builder) {
        this.firstName = Objects.requireNonNull(builder.firstName, "firstName required");
        this.lastName = Objects.requireNonNull(builder.lastName, "lastName required");
        this.email = Objects.requireNonNull(builder.email, "email required");
    }

    public static Builder builder() {
        return new Builder();
    }

    // Getters...
    public String getFirstName() { return firstName; }
    public String getLastName() { return lastName; }
    public String getEmail() { return email; }

    public static class Builder {
        private String firstName;
        private String lastName;
        private String email;

        public Builder firstName(String firstName) {
            this.firstName = firstName;
            return this;
        }

        public Builder lastName(String lastName) {
            this.lastName = lastName;
            return this;
        }

        public Builder email(String email) {
            this.email = email;
            return this;
        }

        public User build() {
            return new User(this);
        }
    }
}

// BAD - no input validation
public void createUser(String name, String email, int age) {
    // Directly uses untrusted input
    User user = new User(name, email, age);
    repository.save(user);
}

// GOOD - validate everything
public void createUser(String name, String email, int age) {
    if (name == null || name.isBlank()) {
        throw new IllegalArgumentException("Name must not be blank");
    }
    if (name.length() > 100) {
        throw new IllegalArgumentException("Name must not exceed 100 characters");
    }
    if (email == null || !EMAIL_PATTERN.matcher(email).matches()) {
        throw new IllegalArgumentException("Invalid email format");
    }
    if (age < 0 || age > 150) {
        throw new IllegalArgumentException("Age must be between 0 and 150");
    }
    User user = new User(name.trim(), email.toLowerCase().trim(), age);
    repository.save(user);
}

private static final Pattern EMAIL_PATTERN =
    Pattern.compile("^[A-Za-z0-9+_.-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,}$");

// BAD - SQL injection vulnerability!
// If username is: ' OR '1'='1' -- , this returns ALL users
public User findUser(String username) {
    String sql = "SELECT * FROM users WHERE username = '" + username + "'";
    return jdbcTemplate.queryForObject(sql, userRowMapper);
}

// GOOD - parameterized query: input is treated as DATA, never as SQL
public User findUser(String username) {
    String sql = "SELECT * FROM users WHERE username = ?";
    return jdbcTemplate.queryForObject(sql, userRowMapper, username);
}

// GOOD - using JPA named parameters
@Query("SELECT u FROM User u WHERE u.username = :username")
Optional findByUsername(@Param("username") String username);

// BAD - secrets in source code (visible in git history forever)
public class DatabaseConfig {
    private static final String DB_PASSWORD = "MyS3cretP@ssw0rd";
    private static final String API_KEY = "sk-1234567890abcdef";
}

// GOOD - read from environment variables
public class DatabaseConfig {
    private final String dbPassword = System.getenv("DB_PASSWORD");
    private final String apiKey = System.getenv("API_KEY");

    public DatabaseConfig() {
        if (dbPassword == null || dbPassword.isBlank()) {
            throw new IllegalStateException(
                "DB_PASSWORD environment variable is not set");
        }
    }
}

// GOOD - Spring Boot: use application.yml with environment variable references
// spring:
//   datasource:
//     password: ${DB_PASSWORD}
// API keys read from a vault or secrets manager in production

// BAD - everything is public (exposes internal implementation)
public class UserService {
    public List userCache = new ArrayList<>();  // internal state exposed
    public Connection dbConnection;                    // anyone can access this

    public String hashPassword(String password) { ... }  // internal method exposed
    public void updateCache(User user) { ... }           // internal method exposed
}

// GOOD - minimal visibility
public class UserService {
    private final List userCache = new ArrayList<>();
    private final DataSource dataSource;

    public UserService(DataSource dataSource) {
        this.dataSource = dataSource;
    }

    // Only public methods that form the API contract
    public User findById(long id) { ... }
    public void register(User user) { ... }

    // Internal helpers are private
    private String hashPassword(String password) { ... }
    private void updateCache(User user) { ... }
}

---