DEV Community

Cover image for The Devil’s Clean Code: Lessons from Migrating a 20-Year-Old Legacy Project
Alberto Talone
Alberto Talone

Posted on

The Devil’s Clean Code: Lessons from Migrating a 20-Year-Old Legacy Project

#programming #java #productivity #learning

TL;DR

  • Write tests. You don't truly realize how messy code is until you try to write a unit test for it.
  • Understand your annotations. Don’t use @Data when @Getter and @Setter are all you need.
  • Don't fear the refactor. If you have the chance to rewrite and simplify, take it.

The Ghost of Architecture Past

In 2025, I took on a daunting challenge: migrating a 20-year-old JSP project into a modern microservice architecture.

To understand the business logic, I had to dive deep into ancient JSP files, trying to decipher what a developer two decades ago was trying to achieve. The client’s initial request sounded simple but proved to be a nightmare: "Copy the models into the new project and transform the beans into services."

The reality was a mess. I found hundreds of model classes scattered across dozens of domain packages. Every package contained at least one "utility" class bloated with static methods for manipulating DAOs and DTOs. The inheritance trees were like Matryoshka dolls—classes extending classes, extending other classes, until the original purpose was lost.

Under pressure to move fast, we made a classic mistake: we copy-pasted the old structures into the new service. We thought converting stateful variables into stateless method parameters would be enough. We were wrong.

Confronting the "Bugfest"

Once the code lived in its new "modern" home, we ran it through SonarQube. It was a disaster.

The report was a "bugfest" of epic proportions: SQL injection vulnerabilities, non-standard naming conventions, and massive blocks of duplicated code. My daily routine became a cycle of frustration: cursing the original developers, questioning my career choices, and ultimately, rolling up my sleeves to clean the mess.

By the end of the cleanup phase, I had:

  • Deleted nearly 2,000 unused classes.
  • Eliminated 30% of duplicated code.
  • Fixed hundreds of bugs and suppressed legacy warnings.

The "microservice" was finally starting to look like one, sitting at 900 classes and 55,000 lines of code. It was time for the final boss: 80% test coverage.

Writing Tests for the "Devil's Clean Code"

Writing tests for legacy code you didn't author is a unique kind of challenge. I leaned heavily on AI assistants to generate test scaffolding, which significantly sped up the process. However, as I looked at the logic I was testing, I realized I was reading the "Devil’s version" of Clean Code.

I encountered:

  • Indentation Hell: Methods with 8 to 10 levels of nested if and for loops.
  • Condition Fatigue: if statements with 10~15 different logic gates.
  • Dead Wood: Massive amounts of unreachable code and unused variables.

Testing forced me to confront these issues. If a branch is impossible to reach in a test, it shouldn't exist in the code.

What I Learned (The Hard Way)

1. Tests are a Diagnostic Tool

Tests are the best way to understand someone else's code. They reveal exactly where the "spaghetti" is. If a method is too hard to test, it’s a signal that the code needs to be rewritten, not just patched.

// BEFORE: The "Pyramid of Doom" (Deeply nested logic)
public void processLegacyRuo(RuoDir ruoDir) {
    if (ruoDir != null) {
        if ("ACTIVE".equals(ruoDir.getTPRuo())) {
            if (ruoDir.getRuoArr() != null) {
                for (RuoItem item : ruoDir.getRuoArr()) {
                    if (item.getGG() > 0) {
                        // Business logic buried 5 levels deep
                        performOperation(item);
                    }
                }
            }
        }
    }
}

// AFTER: Flattened logic using Guard Clauses and Streams
public void processRuo(RuoDir ruoDir) {
    // 1. Exit early if the object is invalid or not in the right state
    if (ruoDir == null || !"ACTIVE".equals(ruoDir.getTPRuo())) {
        return;
    }

    // 2. Handle null collections gracefully
    if (ruoDir.getRuoArr() == null) {
        return;
    }

    // 3. Use Functional Programming to handle the collection
    ruoDir.getRuoArr().stream()
          .filter(item -> item.getGG() > 0)
          .forEach(this::performOperation);
}
Enter fullscreen mode Exit fullscreen mode

2. The Lombok Trap

One of my biggest "Aha!" moments involved Lombok. We initially used @Data on all our models to save time. However, we realized our branch coverage was suffering.

Why? Because @Data automatically generates @EqualsAndHashCode and @ToString. These methods create many hidden logical branches that need to be tested to reach high coverage percentages. By switching to specific @Getter and @Setter annotations, we removed unnecessary complexity and made our coverage targets achievable.

// BEFORE: @Data generates equals, hashCode, and toString, adding hidden branches
@Data
public class UserDto {
    private Long id;
    private String name;
    private String email;
}

// AFTER: Explicit annotations only for what we actually use
@Getter
@Setter
@NoArgsConstructor
public class UserDto {
    private Long id;
    private String name;
    private String email;
}
Enter fullscreen mode Exit fullscreen mode

3. Don't Fear the Rewrite

Legacy code can be intimidating, but you shouldn't be afraid to change it. If you understand the intent of the code, rewrite it for clarity. Your future self (and your team) will thank you.

// BEFORE: A maintenance nightmare with 20+ conditions
if ((input.getDtVersAA() == null || input.getDtVersAA().trim().isEmpty()) && 
    (input.getDtVersMM() == null || input.getDtVersMM().trim().isEmpty()) &&
    // ... imagine 15 more lines of this ...
    (input.getCodFisc() == null || input.getCodFisc().trim().isEmpty())) {

    // Do something if everything is empty
}

// AFTER: Using Method References and Streams for clarity
List<Supplier<String>> fieldsToCheck = Arrays.asList(
        input::getDtVersAA,    input::getDtVersMM, 
        // ... add all 15 more lines ...
        input::getCodFisc
);

// We use a helper method (checkCampoVuoto) and allMatch to verify the state
boolean allFieldsEmpty = fieldsToCheck.stream()
        .allMatch(fieldGetter -> checkCampoVuoto(fieldGetter.get()));

if (allFieldsEmpty) {
    // Logic is now readable and easy to extend
}
Enter fullscreen mode Exit fullscreen mode

Final Thoughts

Migrating legacy systems isn't just about moving code from one place to another; it's about translating old ideas into modern standards. It’s a messy, frustrating, but ultimately rewarding process.

What’s the worst legacy code you’ve ever had to migrate? Let’s swap horror stories in the comments!

Top comments (0)