Diese eine Java-Methode sieht okay aus und ist genau deshalb so gefährlich

Legacy Code ist nicht automatisch schlechter Code. Alt bedeutet erst einmal nur, dass der Code schon lange im Einsatz ist. Das eigentliche Problem beginnt dort, wo niemand mehr genau sagen kann, was eine Methode wirklich tut, welche Seiteneffekte sie hat und welche Fachlogik still und leise an ihr dranhängt. Genau das ist in vielen Java-Anwendungen der Punkt, an dem Änderungen unangenehm werden.

Am Anfang sieht so eine Methode oft harmlos aus. Sie ist lang, aber sie kompiliert. Tests gibt es vielleicht nur wenige oder gar keine. Im Review wirkt alles auf den ersten Blick sauber genug, weil die Methode einen verständlichen Namen hat und am Ende sogar ein Ergebnis zurückliefert. Erst beim genauen Lesen merkst du, dass nebenbei noch ganz andere Dinge passieren.

Eine Methode, die auf den ersten Blick okay aussieht

Schauen wir uns ein typisches Beispiel an. Der Code ist nicht spektakulär. Genau das ist das Problem.

public class OrderService {

    private static int processedOrders = 0;
    private static BigDecimal dailyRevenue = BigDecimal.ZERO;

    private final Map<Long, Integer> customerDiscountCache = new HashMap<>();
    private final List<String> auditLog = new ArrayList<>();

    public BigDecimal processOrder(Order order) {
        BigDecimal total = BigDecimal.ZERO;
        BigDecimal shipping = new BigDecimal("4.99");
        boolean vipCustomer = false;

        if (order == null || order.getItems() == null || order.getItems().isEmpty()) {
            auditLog.add("empty-order");
            return BigDecimal.ZERO;
        }

        if (order.getCustomer() != null && order.getCustomer().isVip()) {
            vipCustomer = true;
        }

        for (OrderItem item : order.getItems()) {
            if (item == null || item.getProduct() == null) {
                continue;
            }

            BigDecimal price = item.getProduct().getPrice();
            int quantity = item.getQuantity();

            if (quantity <= 0) {
                quantity = 1;
            }

            BigDecimal itemTotal = price.multiply(BigDecimal.valueOf(quantity));

            if (vipCustomer) {
                itemTotal = itemTotal.multiply(new BigDecimal("0.95"));
            }

            if (item.getProduct().isFragile()) {
                shipping = shipping.add(new BigDecimal("1.50"));
            }

            total = total.add(itemTotal);
        }

        if (order.getCustomer() != null) {
            Integer cachedDiscount = customerDiscountCache.get(order.getCustomer().getId());
            if (cachedDiscount != null) {
                total = total.subtract(BigDecimal.valueOf(cachedDiscount));
            } else if (total.compareTo(new BigDecimal("100")) > 0) {
                customerDiscountCache.put(order.getCustomer().getId(), 10);
                total = total.subtract(BigDecimal.TEN);
            }
        }

        if (order.isExpress()) {
            shipping = shipping.multiply(new BigDecimal("2"));
            auditLog.add("express-order-" + order.getId());
        }

        total = total.add(shipping);

        if (total.compareTo(BigDecimal.ZERO) < 0) {
            total = BigDecimal.ZERO;
        }

        order.setLastCalculatedTotal(total);
        order.setProcessedAt(LocalDateTime.now());

        processedOrders++;
        dailyRevenue = dailyRevenue.add(total);
        auditLog.add("processed-order-" + order.getId());

        return total;
    }
}

Die Methode sieht im ersten Moment gar nicht so wild aus. Sie berechnet einen Gesamtpreis, berücksichtigt Versand, prüft einen Rabatt und liefert am Ende ein BigDecimal zurück. Wenn du nur den Methodennamen und den Return-Wert anschaust, klingt das nach einer normalen Berechnung.

Das Problem ist nur: Diese Methode berechnet nicht nur etwas. Sie verändert gleichzeitig den Zustand des Order-Objekts, schreibt in ein Audit-Log, verändert einen Rabatt-Cache und erhöht sogar globale Klassenvariablen. Genau diese Mischung macht Legacy Code so unangenehm.

Wo die Schwächen wirklich liegen

Die erste Schwäche sind versteckte Seiteneffekte. Eine Methode mit dem Namen processOrder darf fachlich natürlich mehr tun als nur rechnen. Aber du musst beim Lesen sofort erkennen können, was sie alles verändert. Hier passiert das nicht. Zwischen Schleifen, Bedingungen und Preisregeln gehen die entscheidenden Änderungen fast unter. processedOrders++ und dailyRevenue = dailyRevenue.add(total) wirken klein, haben aber einen großen Effekt. Sie verändern globalen Zustand. Das kann später Auswirkungen auf Statistiken, Reports oder andere Abläufe haben.

Die zweite Schwäche ist die Kopplung von Verantwortung. Preisberechnung, Versandkosten, Rabattlogik, Protokollierung, Cache-Pflege und Statusänderungen am Auftrag liegen alle in derselben Methode. Dadurch wird jede Änderung riskant. Vielleicht willst du nur die Versandkosten anpassen, aber plötzlich schlägt ein Test an einer ganz anderen Stelle fehl. Oder schlimmer: Es gibt keinen Test, und der Fehler taucht erst später in der Fachlogik auf.

Die dritte Schwäche ist die fehlende Transparenz. Die Methode sieht deterministisch aus, ist sie aber nicht vollständig. LocalDateTime.now() bringt Zeit ins Spiel. Der Rabatt hängt vom Cache ab. Das Ergebnis hängt also nicht nur von den Parametern ab, sondern auch vom aktuellen Zustand des Objekts und von statischen Variablen. Genau das macht Debugging so mühsam. Du gibst denselben Auftrag zweimal rein und wunderst dich, warum sich das Verhalten verändert.

Die vierte Schwäche ist der falsche Eindruck von Sicherheit. Legacy-Methoden wie diese laufen oft seit Jahren produktiv. Dadurch entsteht schnell der Gedanke: Dann wird das schon richtig sein. In der Praxis heißt das oft nur, dass viele Sonderfälle und Fachregeln irgendwo in der Methode versteckt überlebt haben. Der Code ist nicht stabil, weil er schön ist, sondern weil sich lange niemand mehr getraut hat, ihn anzufassen.

Warum blindes Refactoring schnell schiefgeht

An genau dieser Stelle machen viele einen typischen Fehler. Sie sehen eine lange Methode und wollen sofort aufräumen. Fachlich ist der Impuls richtig. Technisch ist er ohne Absicherung gefährlich.

Nehmen wir an, du extrahierst einfach die Preisberechnung in eine neue Methode und entfernst dabei scheinbar unnötige Dinge wie das Schreiben ins Audit-Log oder das Setzen von lastCalculatedTotal. Vielleicht denkst du, dass das sowieso nicht in eine Berechnung gehört. Das kann stimmen. Es kann aber genauso gut sein, dass ein anderer Teil der Anwendung genau darauf vertraut. Vielleicht liest ein späterer Prozess processedAt, vielleicht wird der Rabatt-Cache absichtlich an dieser Stelle gefüllt, vielleicht hängt eine Auswertung an dailyRevenue.

Legacy Code ist deshalb so heikel, weil die echte Fachlogik oft nicht nur im sichtbaren Return-Wert steckt. Sie steckt auch in Nebenwirkungen. Genau deshalb solltest du nicht mit Refactoring anfangen, bevor du verstanden hast, welche Teile fachlich relevant sind und welche nur technischer Ballast sind.

Der saubere Weg ist meistens langsamer, aber sicherer. Erst Verhalten beobachten, dann absichern, dann umbauen. Das kann mit charakterisierenden Tests beginnen. Du schreibst also keine idealen neuen Tests für schönen Code, sondern Tests, die das aktuelle Verhalten dokumentieren. Auch dann, wenn dir dieses Verhalten nicht gefällt. Erst wenn du weißt, was heute wirklich passiert, kannst du Änderungen kontrolliert machen.

Wie du es besser machen kannst

Besser wird der Code nicht dadurch, dass du nur die Methode kürzer machst. Besser wird er, wenn Verantwortlichkeiten getrennt und Seiteneffekte sichtbar werden. Eine Preisberechnung sollte möglichst berechnen. Ein Audit sollte möglichst protokollieren. Eine Statusänderung am Auftrag sollte klar erkennbar sein. Und globale Variablen solltest du nur dann anfassen, wenn es wirklich keine bessere Lösung gibt.

Eine mögliche Richtung sieht so aus:

public BigDecimal calculateOrderTotal(Order order) {
    BigDecimal subtotal = priceCalculator.calculateSubtotal(order);
    BigDecimal shipping = shippingCalculator.calculateShipping(order);
    BigDecimal discount = discountService.calculateDiscount(order, subtotal);

    return subtotal.add(shipping).subtract(discount).max(BigDecimal.ZERO);
}

Das ist noch keine komplette Lösung. Aber hier ist sofort klarer, was passiert. Die Methode berechnet einen Wert und verändert nicht nebenbei noch fünf andere Dinge. Wenn du danach den Auftrag aktualisieren oder ein Audit schreiben willst, machst du das sichtbar an anderer Stelle.

Genau darum geht es bei gutem Code in gewachsenen Java-Anwendungen: Verhalten so zu schneiden, dass du es lesen, testen und ändern kannst. Nicht perfekt, nicht akademisch, sondern nachvollziehbar. Du musst nicht jeden Legacy-Block sofort komplett neu bauen. Oft reicht es schon, Seiteneffekte sichtbar zu machen, Abhängigkeiten zu trennen und riskante Stellen zuerst mit Tests abzusichern.

Fazit

Legacy Code ist nicht deshalb ein Problem, weil er alt ist. Er ist dann ein Problem, wenn du beim Lesen nicht mehr sicher sagen kannst, was eine Methode wirklich tut. Lange Methoden mit versteckten Seiteneffekten sind gefährlich, weil sie harmlos aussehen und gleichzeitig mehr verändern, als ihr Name vermuten lässt.

Gerade in Java-Projekten mit gewachsener Fachlogik solltest du deshalb ruhig und systematisch vorgehen. Nicht blind refactorieren, nicht alles auf einmal anfassen und nicht davon ausgehen, dass ein schönerer Code automatisch dieselbe Fachlogik behält. Verstehe erst das aktuelle Verhalten, sichere es ab und verbessere dann Schritt für Schritt die Struktur. Genau so machst du aus schwer verständlichem Legacy Code nach und nach wieder Code, mit dem man vernünftig arbeiten kann.