Die häufigsten Fallstricke, auf die Entwickler während Code-Reviews stoßen, mit einem Fokus auf Strategien zur Verbesserung des Review-Prozesses. Dieser Artikel richtet sich an Entwickler, die nach Möglichkeiten suchen, ihren Workflow zu verbessern und ihre Pull Request schneller genehmigt zu bekommen.

Während viele Entwickler die Grundlagen gut beherrschen, gibt es subtile, oft übersehene Aspekte, die leicht durch die Maschen fallen.

Obwohl der Code wie erwartet funktioniert, sind es oft genau diese subtilen Aspekte, die später zu Ineffizienzen, Performance-Problemen oder Bugs führen können. Dieser Artikel konzentriert sich auf 5 übersehene Probleme, die Entwickler vor der Einreichung eines PRs noch einmal überprüfen sollten. Wenn man sich damit auseinandersetzt, kann dies die Qualität, Wartbarkeit und Performance des Codes erheblich verbessern.

In diesem Artikel möchte ich mich auf diese weniger offensichtlichen, aber ebenso wichtigen Aspekte konzentrieren – auf Nuancen, die über die üblichen Punkte wie ‚Tests schreiben‘ oder ‚Null-Werte prüfen‘ hinausgehen.

Lassen Sie uns die fünf Dinge durchgehen, die Sie unbedingt vor der Einreichung Ihres nächsten Pull Request überprüfen sollten, um diese versteckten Fallstricke zu vermeiden.

1. Verborgene Performance-Engpässe bei Stream-Operationen

Java Streams bieten eine elegante, deklarative Methode, um Daten zu verarbeiten. Sie sind eine ausgezeichnete Wahl, um sauberen und gut lesbaren Code zu schreiben. Dennoch können einige Stream-Operationen, wie etwa .distinct() oder .sorted(), Performance-Probleme verursachen, wenn sie auf große Datenmengen angewendet werden oder unnötig verschachtelt sind.

Ein häufiges Beispiel für einen ineffizienten Einsatz von Streams könnte folgender Code sein:

List<String> names = List.of("John", "Jane", "John", "Anna");
List<String> distinctNames = names.stream()
    .filter(name -> name.length() > 3)
    .distinct()  // Hier unnötig, wenn Duplikate irrelevant sind
    .collect(Collectors.toList());

In diesem Fall wird die Methode .distinct() aufgerufen, um doppelte Einträge zu entfernen. Aber wenn wir sicher wissen, dass die Liste names keine Duplikate enthält oder Duplikate irrelevant sind, führt diese Methode zu unnötigem Overhead. Dieser Overhead kann bei größeren Datensätzen besonders problematisch werden, da .distinct() eine zusätzliche Kosten für das Vergleichen und Entfernen von Duplikaten mit sich bringt.

Was sollten Sie vor der Einreichung eines Pull Request überprüfen?

• Sind Sie sicher, dass Sie nur dann Operationen wie .distinct(), .sorted() oder .collect() verwenden, wenn es wirklich notwendig ist?
  Oft fügen Entwickler diese Methoden hinzu, ohne zu hinterfragen, ob sie tatsächlich zur Logik des Programms beitragen. Es kann sein, dass Sie zusätzliche Operationen ausführen, die keine Verbesserung der Funktionalität bieten und lediglich die Performance beeinträchtigen.
• Könnte der Einsatz eines parallelen Streams bei großen Datensätzen die Performance verbessern?
  Ein paralleler Stream (parallelStream()) kann helfen, die Verarbeitung großer Datenmengen zu beschleunigen, indem er die Last auf mehrere Threads verteilt. Doch Vorsicht: Parallelisierung kann in manchen Fällen auch zu einer schlechteren Performance führen, wenn die Datenmenge zu klein oder die Operation zu aufwendig ist.

Optimiertes Beispiel:

List<String> distinctNames = names.stream()
    .filter(name -> name.length() > 3)
    .collect(Collectors.toList());  // Unnötiges `distinct()` entfernt

In diesem optimierten Beispiel haben wir die .distinct()-Methode entfernt, da sie keinen Mehrwert bringt und nur unnötige Ressourcen verbraucht. Stattdessen konzentrieren wir uns darauf, nur die Operationen durchzuführen, die wirklich erforderlich sind, um das gewünschte Ergebnis zu erzielen.

Beim Arbeiten mit Streams in Java sollten Sie immer kritisch hinterfragen, ob jede verwendete Operation wirklich notwendig ist. Unnötige Stream-Methoden, selbst solche, die auf den ersten Blick harmlos wirken, können die Performance erheblich beeinträchtigen, insbesondere wenn Sie mit großen Datensätzen oder komplexen Datenstrukturen arbeiten. Daher ist es wichtig, die Auswirkungen jeder Stream-Operation auf die Performance zu verstehen und nur dann anzuwenden, wenn es wirklich erforderlich ist.

2. Ineffiziente Nutzung von Synchronisation in hochgradig konkurrierendem Code

Concurrency (Nebenläufigkeit) ist ein komplexes Thema, und Fehler in diesem Bereich können gravierende Auswirkungen auf die Performance einer Anwendung haben. Synchronisation ist ein wichtiges Konzept, um sicherzustellen, dass mehrere Threads korrekt auf gemeinsam genutzte Ressourcen zugreifen, ohne Konflikte oder Inkonsistenzen zu verursachen. Allerdings wird Synchronisation oft nicht optimal eingesetzt, was zu schwerwiegenden Performance-Problemen führen kann. Ein häufiger Fehler ist das Über-Synchronisieren von Methoden, d.h., mehr Code wird gesperrt, als eigentlich nötig wäre.

Ein einfaches Beispiel, um diesen Fehler zu verdeutlichen:

public synchronized void updateOrder(Order order) {
    // Aktualisieren der Bestelldaten in der Datenbank
    database.update(order);
    // Loggen der Aktualisierung (dieser Teil muss nicht synchronisiert werden)
    logger.info("Order updated: " + order.getId());
}

In diesem Fall ist die gesamte Methode updateOrder als synchronized deklariert. Das bedeutet, dass nur ein Thread gleichzeitig in der Lage ist, diese Methode auszuführen. Doch das ist nicht unbedingt notwendig. Der kritische Abschnitt ist nur das Aktualisieren der Datenbank, während das Loggen der Aktualisierung – das Schreiben von Informationen in ein Log – nicht auf dieselbe Weise synchronisiert werden muss. Das bedeutet, dass der Logging-Teil der Methode unnötig gesperrt wird, was zu einer geringeren Parallelität und damit zu einer schlechteren Performance führen kann.

Warum ist das ein Problem?
Das Synchronisieren von Codeblocken kostet Zeit und Ressourcen. Wenn Sie mehr Code synchronisieren, als wirklich erforderlich ist, blockieren Sie unnötig andere Threads, die auf diesen Code zugreifen wollen. Dies kann die Performance erheblich beeinträchtigen, besonders bei hochgradig parallelen Anwendungen, in denen viele Threads gleichzeitig ausgeführt werden.

Was sollten Sie überprüfen beim Pull Request:

• Synchronisieren Sie mehr Code, als notwendig ist?
  Häufig wird die Synchronisation zu weit gefasst eingesetzt, beispielsweise auf gesamte Methoden oder große Codeblöcke. Es ist wichtig, nur die wirklich kritischen Abschnitte zu synchronisieren, in denen mehrere Threads auf gemeinsam genutzte Ressourcen zugreifen und Daten inkonsistent werden könnten.
• Können Sie nicht-kritische Teile der Methode außerhalb des synchronisierten Blocks auslagern?
  Wenn nur bestimmte Teile des Codes wirklich synchronisiert werden müssen, dann sollten Sie den nicht-kritischen Code davon trennen und außerhalb des synchronisierten Blocks ausführen. Das erhöht die Parallelität und minimiert die Sperrzeit.

Optimiertes Beispiel:

public void updateOrder(Order order) {
    // Nur der kritische Abschnitt wird synchronisiert
    synchronized (this) {
        database.update(order);
    }
    // Dieser Teil läuft außerhalb des Lock-Blocks
    logger.info("Order updated: " + order.getId());
}

In diesem optimierten Beispiel wird nur der wirklich kritische Abschnitt der Methode synchronisiert, nämlich die Aktualisierung der Bestelldaten in der Datenbank. Das Loggen wird von der Synchronisation ausgenommen und läuft außerhalb des synchronized-Blocks. Dadurch wird die Zeit, in der der Lock gehalten wird, erheblich verkürzt, und andere Threads können gleichzeitig andere Aufgaben ausführen, ohne auf den Lock warten zu müssen.

Alternative Ansätze:

In bestimmten Fällen kann es sinnvoll sein, Alternativen zur herkömmlichen Synchronisation zu verwenden, um Engpässe zu vermeiden:

• ReadWriteLock: Wenn Sie ein Szenario haben, in dem viele Lesezugriffe, aber nur wenige Schreibzugriffe auf eine Ressource stattfinden, könnte ein ReadWriteLock eine bessere Lösung sein. Mit einem ReadWriteLock können mehrere Threads gleichzeitig auf eine Ressource zugreifen, solange sie nur lesen, aber Schreiboperationen werden weiterhin exklusiv behandelt. Dies kann die Parallelität in einem System mit vielen Lesezugriffen erheblich verbessern.
• AtomicReference und ähnliche Klassen: Für einfache Operationen, bei denen es um die Änderung eines einzelnen Wertes geht, bieten sich atomare Referenzen oder andere atomare Datentypen an. Diese ermöglichen es, Operationen sicher und ohne explizite Synchronisation durchzuführen. Klassen wie AtomicInteger, AtomicLong oder AtomicReference bieten eine effiziente Möglichkeit, die Synchronisation von einfachen Datenoperationen zu vermeiden.

Die richtige Anwendung von Synchronisation ist entscheidend für die Performance und Effizienz von nebenläufigem Code. Synchronisation sollte immer gezielt und nur für die kritischen Abschnitte eingesetzt werden. Zu viele Synchronisationspunkte können die Performance negativ beeinflussen, indem sie Threads unnötig blockieren und so die Parallelität einschränken. Eine enge Begrenzung der synchronisierten Bereiche und der Einsatz alternativer Konzepte wie ReadWriteLock oder atomare Referenzen können helfen, Engpässe zu minimieren und die Performance zu optimieren.

3. Vernachlässigung der Speicherimplikationen von Lambdas und anonymen Klassen

Lambdas und anonyme Klassen bieten eine kompakte und elegante Möglichkeit, Funktionalität in Java zu implementieren. Allerdings können sie unbeabsichtigte Speicherüberlastungen verursachen. Ein häufiges Problem tritt auf, wenn diese Konstrukte Referenzen auf umschließende Variablen (also Variablen aus dem äußeren Gültigkeitsbereich) erfassen. Dies kann dazu führen, dass diese Variablen im Speicher bleiben und die Garbage Collection nicht in der Lage ist, den Speicher freizugeben, was im schlimmsten Fall zu Speicherlecks führen kann.

Beispiel: Erfassen von Variablen aus dem äußeren Gültigkeitsbereich

public Runnable createRunnable(String taskName) {
    return () -> System.out.println("Executing: " + taskName);
}

In diesem Beispiel erfasst die Lambda-Ausdruck die taskName-Variable, die aus dem äußeren Gültigkeitsbereich stammt. Wenn der zurückgegebene Runnable lange lebt – etwa in einem Hintergrund-Thread oder Service – könnte die taskName-Variable im Speicher verbleiben, selbst nachdem sie nicht mehr benötigt wird. Dies kann unnötigen Speicherverbrauch verursachen und in extremen Fällen zu Speicherlecks führen, wenn der Runnable nie zur Garbage Collection freigegeben wird.

Was sollten Sie beim Pull Request überprüfen:

• Erfassen Ihre Lambdas oder inneren Klassen unnötigerweise äußere Variablen?
  Wenn Lambdas oder anonyme Klassen auf Variablen aus dem äußeren Gültigkeitsbereich zugreifen, dann wird diese Variable durch die Lambda oder anonyme Klasse „eingeschlossen“ (captured). Dies kann dazu führen, dass der Speicher, den diese Variablen benötigen, länger als nötig gehalten wird. Überlegen Sie, ob es wirklich notwendig ist, dass die Lambda-Ausdrücke oder anonymen Klassen diese Variablen erfassen oder ob es eine Möglichkeit gibt, dies zu vermeiden.
• Bleiben Closures länger als erwartet im Speicher?
  Ein weiteres Problem tritt auf, wenn Closures (also die Kombination aus Lambda-Ausdrücken und ihren erfassten Variablen) in langlebigen Prozessen wie Threads oder Services verwendet werden. In solchen Fällen bleibt der erfasste Kontext möglicherweise unnötig lange im Speicher, was zu einem erhöhten Speicherverbrauch führen kann.

Optimiertes Beispiel:

public Runnable createRunnable(String taskName) {
    String finalTaskName = new String(taskName);  // Vermeidet das Erfassen der äußeren Variablen
    return () -> System.out.println("Executing: " + finalTaskName);
}

In diesem optimierten Beispiel haben wir eine Kopie der taskName-Variablen erstellt (durch die Verwendung des new String(taskName)), sodass die Lambda-Expression nicht mehr auf die äußere taskName-Variable verweist. Stattdessen arbeitet die Lambda mit der lokalen Kopie finalTaskName, was verhindert, dass die äußere Variable unnötig im Speicher bleibt.

Lambdas und anonyme Klassen sind äußerst nützlich, aber sie erfordern eine sorgfältige Handhabung, insbesondere wenn sie Variablen aus dem äußeren Gültigkeitsbereich erfassen. Solche erfassten Variablen können im Speicher gehalten werden und verhindern, dass der Speicher ordnungsgemäß freigegeben wird, was zu Speicherlecks führen kann. Es ist wichtig, Lambdas und anonyme Klassen regelmäßig zu überprüfen, insbesondere in langlebigen Prozessen wie Threads oder Services, und sicherzustellen, dass sie keine unnötigen Referenzen auf äußere Variablen halten.

4. Unbeabsichtigte späte Initialisierung im Singleton-Pattern

Späte Initialisierung (Lazy Initialization) ist ein häufig verwendetes Muster, um Objekte erst dann zu erstellen, wenn sie tatsächlich benötigt werden. Es bietet Vorteile in Bezug auf Performance, indem Ressourcen nur bei Bedarf angefordert werden. Doch wenn dieses Muster nicht richtig umgesetzt wird, kann es subtile Probleme mit der Thread-Sicherheit verursachen. In einer multithreaded Umgebung können mehrere Threads gleichzeitig versuchen, eine Instanz einer Klasse zu erstellen, was zu unerwarteten Ergebnissen führen kann.

Beispiel: Fehlerhafte späte Initialisierung

public class OrderService {
    private static OrderService instance;
    
    public static OrderService getInstance() {
        if (instance == null) {
            instance = new OrderService();  // Nicht thread-sicher
        }
        return instance;
    }
}

In diesem Beispiel wird eine Singleton-Instanz von OrderService durch späte Initialisierung erstellt. Das Problem hier ist, dass der Code nicht thread-sicher ist. In einer Umgebung mit mehreren Threads könnten mehrere Threads gleichzeitig in den Block if (instance == null) eintreten und versuchen, eine Instanz zu erstellen. Dadurch könnten mehrere Instanzen der OrderService-Klasse entstehen, was das Singleton-Muster bricht und zu unerwarteten Fehlern führt.

Was sollten Sie überprüfen beim Pull Request:

• Ist Ihr Lazy-Initialization-Muster thread-sicher?
  Wenn Sie späte Initialisierung verwenden, stellen Sie sicher, dass der Code korrekt synchronisiert wird, um Probleme mit der gleichzeitigen Erstellung mehrerer Instanzen zu vermeiden. Ohne entsprechende Synchronisation können konkurrierende Threads mehrere Instanzen der Klasse erzeugen, was nicht dem Singleton-Muster entspricht.
• Könnten Sie Alternativen wie das Bill-Pugh-Singleton oder Enum-Singletons verwenden?
  Es gibt gut etablierte Muster, die späte Initialisierung in einer thread-sicheren Weise durchführen. Diese Muster vermeiden die Komplexität und die Risiken, die mit der manuellen Synchronisation verbunden sind.

Optimiertes Beispiel: Double-Checked Locking

public class OrderService {
    private static volatile OrderService instance;
    
    public static OrderService getInstance() {
        if (instance == null) {
            synchronized (OrderService.class) {
                if (instance == null) {
                    instance = new OrderService();  // Double-Checked Locking
                }
            }
        }
        return instance;
    }
}

In diesem optimierten Beispiel wird Double-Checked Locking verwendet, um das Problem der mehrfachen Instanzenerstellung zu verhindern und die Performance zu verbessern. Hier wird die Instanz nur dann erstellt, wenn sie noch nicht vorhanden ist, und die Synchronisation erfolgt nur einmal, wenn dies unbedingt erforderlich ist. Der Schlüssel dabei ist die Verwendung des volatile-Modifiers für die Instanzvariable. Dieser sorgt dafür, dass Änderungen an der instance-Variable korrekt zwischen Threads synchronisiert werden und dass keine Instanz mehrfach erstellt wird.

Alternative: Bill-Pugh-Singleton

Eine andere, noch sicherere und elegantere Lösung für das Singleton-Pattern ist die Verwendung des Bill-Pugh-Singletons. Es nutzt die Tatsache, dass statische Initialisierung in Java garantiert thread-sicher ist:

public class OrderService {
    private OrderService() {}

    private static class SingletonHelper {
        private static final OrderService INSTANCE = new OrderService();
    }

    public static OrderService getInstance() {
        return SingletonHelper.INSTANCE;
    }
}

In diesem Fall wird die OrderService-Instanz nur beim ersten Zugriff auf die SingletonHelper-Klasse erstellt, was ebenfalls eine thread-sichere späte Initialisierung gewährleistet und dabei die Synchronisation vermeidet.

Späte Initialisierung ist ein mächtiges Konzept, aber wenn es um Singleton-Patterns geht, muss die Thread-Sicherheit unbedingt berücksichtigt werden. Der klassische Ansatz der synchronisierten Blockierung kann ineffizient sein und Performance-Probleme verursachen, wenn er nicht korrekt implementiert ist. Alternativen wie Double-Checked Locking oder das Bill-Pugh-Singleton sind bewährte Muster, die sowohl thread-sicher als auch effizient sind. Achten Sie stets darauf, dass Ihre Implementierungen in einer Umgebung mit mehreren Threads korrekt und sicher arbeiten. Deshalb sollten Sie spätestens beim Pull Request darauf schauen.

5. Verwirrung zwischen Überladen und Überschreiben in der Vererbung

Die Begriffe Überladen (Overloading) und Überschreiben (Overriding) werden oft verwechselt, besonders in Vererbungshierarchien. Ein Missverständnis der beiden Konzepte kann zu schwer zu entdeckenden Fehlern führen. Es ist wichtig, den Unterschied zwischen diesen beiden zu verstehen, um unerwünschte Nebeneffekte und Bugs zu vermeiden.

Beispiel: Verwechslung von Überladen und Überschreiben

public class BaseService {
    public void processOrder(Order order) {
        System.out.println("Processing base order.");
    }
}

public class SpecialService extends BaseService {
    public void processOrder(SpecialOrder order) {  // Überladen, nicht Überschreiben
        System.out.println("Processing special order.");
    }
}

In diesem Beispiel wird die Methode processOrder(SpecialOrder order) in der SpecialService-Klasse überladen, nicht überschrieben. Beim Überladen wird eine Methode mit einem anderen Parameter in der abgeleiteten Klasse definiert, während das Überschreiben bedeutet, dass eine Methode der Basisklasse mit exakt der gleichen Signatur überschrieben wird. Wenn nun processOrder() auf einem Objekt der Klasse SpecialService aufgerufen wird, könnte versehentlich die Methode der Basisklasse aufgerufen werden, da der Parameter der Methode nicht mit dem in der Basisklasse übereinstimmt. Das führt zu ungewolltem Verhalten.

Was sollten Sie überprüfen beim Pull Request:

• Überschreiben Ihre Methoden korrekt die Methoden der Superklasse?
  Achten Sie darauf, dass Sie beim Überschreiben einer Methode die exakt gleiche Methodensignatur verwenden wie in der Basisklasse. Wenn Sie die Signatur ändern (z.B. durch einen anderen Parametertyp), handelt es sich nicht um ein Überschreiben, sondern um ein Überladen.
• Stimmt die Methodensignatur exakt überein, wenn Sie eine Methode überschreiben?
  Um sicherzustellen, dass Sie tatsächlich überschreiben und nicht überladen, muss die Methodensignatur (Methodenname und Parameterliste) der in der Basisklasse exakt entsprechen.

Optimiertes Beispiel:

public class SpecialService extends BaseService {
    @Override
    public void processOrder(Order order) {  // Überschreibt korrekt die Methode der Basisklasse
        if (order instanceof SpecialOrder) {
            System.out.println("Processing special order.");
        } else {
            super.processOrder(order);  // Aufruf der Methode der Basisklasse für normale Bestellungen
        }
    }
}

In diesem optimierten Beispiel überschreibt die Methode processOrder in der Klasse SpecialService die Methode der Basisklasse BaseService korrekt. Der @Override-Annotation wird verwendet, um sicherzustellen, dass die Methode wirklich die Methode der Basisklasse überschreibt und keine neue Methode überladen wird. Wenn die Bestellung eine spezielle Bestellung (SpecialOrder) ist, wird eine spezifische Logik angewendet, andernfalls wird die Methode der Basisklasse durch den Aufruf von super.processOrder(order) verwendet.

Die Verwechslung von Überladen und Überschreiben kann zu subtilen Fehlern führen, die schwierig zu finden sind. Wenn Sie Methoden überschreiben, stellen Sie sicher, dass die Methodensignatur genau mit der der Basisklasse übereinstimmt, und verwenden Sie stets die @Override-Annotation, um versehentliches Überladen zu vermeiden. So stellen Sie sicher, dass Ihr Code korrekt funktioniert und verhindern unerwartete Fehler bei der Ausführung.

Fazit

Das Einreichen eines Pull Request geht über das Schreiben von funktionalem Code hinaus. Indem Sie versteckte Performance-Engpässe überprüfen, die Synchronisation optimieren, den Speicherverbrauch bei Lambdas verwalten, thread-sichere späte Initialisierung sicherstellen und den Unterschied zwischen Überladen und Überschreiben verstehen, können Sie die Qualität und Stabilität Ihres Codes erheblich verbessern.

Überprüfen Sie diese fünf oft übersehenen Bereiche, bevor Sie Ihren PR einreichen, um schwer zu debuggende Probleme in der Produktion zu vermeiden. Diese subtilen Anpassungen können einen großen Unterschied sowohl für den kurzfristigen als auch den langfristigen Erfolg Ihrer Software ausmachen.