Common Coding Mistakes Even Experienced Developers Make

Good Developers Avoid These Subtle Mistakes

In this post we will discuss some less-discussed code and design “smells” - things that might appear harmless but can cause maintainability or testing headaches down the line.

---

Making Everything “Private + Getter/Setter” by Default

What’s the problem?

• A common Java pattern is to mark all fields private, then provide getters and setters for every field. But this can lead to “Property Blob” classes, where every internal detail is exposed via getters/setters.

• Overusing getters/setters can break encapsulation (the point of making things private in the first place).

• Often, you only need to expose a subset of the fields, or provide methods that encapsulate behavior instead of just exposing the data.

  Bad Example

  public class User {
      private String name;
      private String email;
      private int age;
  
      // Blanket getters and setters for all fields
      public String getName() { return name; }
      public void setName(String name) { this.name = name; }
      
      public String getEmail() { return email; }
      public void setEmail(String email) { this.email = email; }
      
      public int getAge() { return age; }
      public void setAge(int age) { this.age = age; }
      
      // ... potential logic that requires data validation ...
  }

  Better Approach

  • Only provide getters/setters that make sense for your business logic.

  • Consider replacing “setters” with methods that perform meaningful actions and validations.

  Improved Example

  public class User {
      private String name;
      private String email;
      private int age;
  
      public User(String name, String email, int age) {
          // Possibly validate name, email, and age here
          this.name = name;
          this.email = email;
          this.age = age;
      }
  
      public String getName() { return name; }
      
      public String getEmail() { return email; }
      
      public int getAge() { return age; }
  
      // Instead of a generic setAge, we have a method that actually handles the logic:
      public void celebrateBirthday() {
          this.age++;
      }
  }

  This class focuses on meaningful operations rather than exposing its entire internal state.

---

Excessive Use of Static Methods and Variables

What’s the problem?

• Static variables are effectively “global variables.” They create coupling throughout the code and are difficult to mock or substitute in tests.

• Static methods are fine for pure utility functions (e.g., Math.max), but if they rely on external resources or maintain state, you can’t easily inject dependencies or override behavior.

Bad Example

public class GlobalConfig {
    public static String dbUrl = "jdbc:mysql://localhost/mydb";

    public static void connect() {
        // Connects using dbUrl
    }
}

• Any part of the code can modify dbUrl, making debugging difficult.

• Testing different database URLs in parallel is nearly impossible.

Better Approach

• Use dependency injection or pass the relevant configuration where needed.

• Keep utility methods pure (no state or external resource coupling).

Improved Example

public class DatabaseConnector {
    private final String dbUrl;

    public DatabaseConnector(String dbUrl) {
        this.dbUrl = dbUrl;
    }

    public Connection connect() throws SQLException {
        return DriverManager.getConnection(dbUrl);
    }
}

// Usage:
DatabaseConnector connector = new DatabaseConnector("jdbc:mysql://localhost/mydb");
Connection conn = connector.connect();

Now we can test with different dbUrl values without global state.

---

Overusing Enums for Behavior Instead of Polymorphism

Why it's a problem?

• Using enums to drive behavior can lead to long switch statements or if-else ladders, making the code harder to extend.

• Every time you add a new enum value, you must update all switch cases, violating the Open-Closed Principle (OCP).

• If enums start including logic (methods), they start behaving like pseudo-classes but lack real extensibility.

Bad Example

enum PaymentType {
    CREDIT_CARD, PAYPAL, APPLE_PAY;
}

class PaymentProcessor {
    public void processPayment(PaymentType type, double amount) {
        switch (type) {
            case CREDIT_CARD:
                System.out.println("Processing credit card payment of $" + amount);
                break;
            case PAYPAL:
                System.out.println("Processing PayPal payment of $" + amount);
                break;
            case APPLE_PAY:
                System.out.println("Processing Apple Pay payment of $" + amount);
                break;
            default:
                throw new IllegalArgumentException("Unknown payment type");
        }
    }
}

• If a new payment method is added (e.g., GOOGLE_PAY), you must modify the processPayment method, violating OCP.

• The switch case makes the code harder to maintain.

Better Approach – Use Polymorphism (Strategy Pattern)

• Instead of an enum, define an interface with separate implementations for each payment type.

Improved Example

interface PaymentMethod {
    void process(double amount);
}

class CreditCardPayment implements PaymentMethod {
    public void process(double amount) {
        System.out.println("Processing credit card payment of $" + amount);
    }
}

class PayPalPayment implements PaymentMethod {
    public void process(double amount) {
        System.out.println("Processing PayPal payment of $" + amount);
    }
}

class ApplePayPayment implements PaymentMethod {
    public void process(double amount) {
        System.out.println("Processing Apple Pay payment of $" + amount);
    }
}

// Client code
public class PaymentProcessor {
    public static void main(String[] args) {
        PaymentMethod payment = new CreditCardPayment();
        payment.process(100.0);
    }
}

Why this is better?

Easier to extend – You can add new payment methods without modifying existing code.
No need for switch cases – Each payment method encapsulates its own logic.
More testable – You can test each class separately.

---

Using Boolean Flags to Control Multiple Behaviors

Why it's a problem?

• Using a boolean flag to determine different behaviors inside a method can make code hard to understand and difficult to maintain.

• When more behaviors are added, you end up with multiple booleans, creating complex logic paths.

Bad Example

class ReportGenerator:
    def generate(self, is_summary: bool):
        if is_summary:
            print("Generating summary report...")
            # Summary report logic
        else:
            print("Generating detailed report...")
            # Detailed report logic

Problems:

• The generate method does two different things based on a flag, violating Single Responsibility Principle.

• If more flags are added (is_pdf=True, is_secure=True), it turns into flag explosion.

• Adding new report types would require modifying the same method, breaking Open-Closed Principle.

Better Approach – Use Separate Methods or Classes

class SummaryReport:
    def generate(self):
        print("Generating summary report...")
        # Summary report logic

class DetailedReport:
    def generate(self):
        print("Generating detailed report...")
        # Detailed report logic

# Usage
report = SummaryReport()
report.generate()

Easier to read and maintain – No need to check what is_summary does.
Extensible – If you need another type (e.g., HTMLReport), you just add a new class.
Encapsulates behavior properly – Each class is responsible for its own logic.

---

Overuse of Private Methods Hiding Complexity

What’s the problem?

• Having a few private methods is normal for breaking down large public methods. However, when a class has a large number of private methods, it often signals that the class is doing too much.

• Complex private methods make testing harder—most testing frameworks can’t directly test private methods without reflection, so you end up either skipping important tests or resorting to hacky solutions.

• It can violate the Single Responsibility Principle if the class has many responsibilities hidden away in private methods.

Bad Example

public class OrderProcessor {
    public void processOrder(Order order) {
        // Some high-level process
        validateOrder(order);
        applyDiscount(order);
        calculateTaxes(order);
        updateInventory(order);
        sendReceipt(order);
    }

    private void validateOrder(Order order) { /* Complex validation logic */ }
    private void applyDiscount(Order order) { /* Complex discount logic */ }
    private void calculateTaxes(Order order) { /* Complex tax logic */ }
    private void updateInventory(Order order) { /* Complex update logic */ }
    private void sendReceipt(Order order) { /* Complex communication logic */ }
}

Each private method might be hundreds of lines if the logic is genuinely complex.

Better Approach

• Split out the different responsibilities into collaborating classes or services.

• Keep private methods only for small, helper-like logic that’s truly specific to that class.

Improved Example

public class OrderProcessor {
    private final OrderValidator validator;
    private final DiscountService discountService;
    private final TaxCalculator taxCalculator;
    private final InventoryManager inventoryManager;
    private final NotificationService notificationService;

    public OrderProcessor(
        OrderValidator validator,
        DiscountService discountService,
        TaxCalculator taxCalculator,
        InventoryManager inventoryManager,
        NotificationService notificationService
    ) {
        this.validator = validator;
        this.discountService = discountService;
        this.taxCalculator = taxCalculator;
        this.inventoryManager = inventoryManager;
        this.notificationService = notificationService;
    }

    public void processOrder(Order order) {
        validator.validate(order);
        discountService.applyDiscount(order);
        taxCalculator.calculateTaxes(order);
        inventoryManager.updateInventory(order);
        notificationService.sendReceipt(order);
    }
}

• Here, each piece of functionality is in its own class, which can be tested independently.

• We still might have small private helpers in OrderProcessor, but we avoid burying large, critical logic inside private methods.

---

Making Your Class Too Large

What’s the problem?

• A God Object is a class that knows too much or does too much.

• It’s hard to maintain or test because you have to load a giant context to verify any small piece of functionality.

• Often leads to a tangle of private helper methods (as mentioned in #1) or many getters and setters for everything in a single place.

public class MegaController {
    // Fields for multiple concerns
    private PaymentService paymentService;
    private InventoryManager inventoryManager;
    private ShippingService shippingService;
    private UserService userService;
    private AnalyticsTracker analyticsTracker;
    private EmailService emailService;
    // ... and so on ...

    // doCheckout, doAddToCart, doUserLogin, doSendNewsletter, doAnalyticsTracking,
    // handleUserProfileUpdates, etc. All in one monster class.
}

• The class orchestrates multiple unrelated functionalities.

Better Approach

• Apply the Single Responsibility Principle: each class should have one reason to change.

• Break up concerns into smaller classes or modules.

Improved Example

CartController        -> Manages cart operations
CheckoutController    -> Handles checkout flow
UserController        -> Manages user login/profile
AnalyticsController   -> Tracks analytics
NotificationService   -> Sends newsletters/emails

Each class has a focused responsibility, making them easier to maintain, test, and extend.

---

Testing Private Methods Directly

What’s the problem?

• Private methods are implementation details. If you find yourself wanting to test them directly, it may mean your class is doing too much.

• Tests that rely on private methods can break if you refactor your implementation- even if the overall behavior (public API) remains correct.

Bad Example

public class StringProcessorTest {

    @Test
    public void testPrivateMethod() throws Exception {
        StringProcessor processor = new StringProcessor();
        Method hiddenMethod = StringProcessor.class
            .getDeclaredMethod("privateLogic", String.class);
        hiddenMethod.setAccessible(true);

        String result = (String) hiddenMethod.invoke(processor, "input");
        assertEquals("processed input", result);
    }
}

• This approach is brittle. You’re reaching into the class’s internals using reflection.

Better Approach

• Test the public methods which use the private methods internally.

• If a private method is truly complex and important, consider extracting it into a separate class or utility that can be tested on its own.

Improved Example

// Instead of testing privateLogic directly, we call the public method:
public class StringProcessorTest {

    @Test
    public void testProcessString() {
        StringProcessor processor = new StringProcessor();
        String result = processor.process("input");
        assertEquals("processed input", result);
    }
}

Here, process might internally call privateLogic. If privateLogic changes, as long as process still produces the correct result, this test remains valid.

---

Many of these “smells” are subtle - they don’t necessarily break your code immediately, but they can make it more fragile, harder to test, and difficult to evolve.

Don’t be afraid to refactor away from these patterns, even if they seem “minor,” because technical debt grows exponentially over time.