Aus alt mach Neu - Legacy Code modernisieren

Jeder kennt es. Software wächst und wächst und es entstehen immer neue Anforderungen. Dabei bleiben technische Schulden leider oft auf der Strecke. Wie geht es nun weiter? Die Legacy-Codebasis komplett ersetzen und von neu Programmieren? Das wird extrem schwer eurem Product Owner zu erklären.

In diesem Talk möchte ich euch zeigen, wie bei uns ein zentrales Modul modernisiert wurde. Ein paar Eckdaten: 3900 Zeilen Code, 19 Abhängigkeiten und eine NPath Komplexität jenseits von gut und böse. Und das alles natürlich in nur einer der dazugehörigen Klassen. Hier wird gezeigt wie man so ein Projekt umsetzen kann, sodass Entwickler und Product Owner zufrieden sind. Die Software wird modernisiert und neue Funktionen können während der Umbaumaßnahmen implementiert werden.

Habt keine Angst mehr vor eurem alten, ungetesteten und fehleranfälligen Code. Beginnt Ihn zu modernisieren!
    

Aus alt mach Neu

Legacy Code modernisieren

Aus alt mach Neu

Ausgangssituation

Search Result Handler

  • Erstellt im August 2015
  • > 20 Entwickler
  • 4 miteinander verschmolzene Klassen
  • Testabdeckung gegen 0

SRH - Aufgaben

  • Startet Suchen
  • Verarbeitet Suchen
  • Verarbeitet Filter
  • Erstellt Filter Zusammenfassung
  • Rendert Ergebnisse für Listen
  • Rendert Ergebnisse für Karten
  • Rendert Ergebnisse für Detailansichten
  • Übernimmt das Caching der Suchen

SRH - Datenbank Ablaufdiagramm

Anforderungen analysieren

Grobe Anforderungen

  • Modular
  • Testabdeckung
  • Performant
  • Pflegbar
  • Fehlerfrei
  • Dynamisch
  • Konfigurierbar
B I N G O
Perfomant ASAP Agil Modular Tests
Pflegbar wenig Aufwand Sicher Microservice Dokumentation
Kosten Anforderungen Kundenorientiert Conversion Rate Qualität
Fehlerfrei Pflegbar Dynamisch Konfigurierbar Stabilität
Technisch sauber Zukunftsfähig Flexibilität Schnell Testabdeckung

Detailierte Anforderungen

  • 3-stündiges Meeting mit Produktmanagement
  • Aktuelle und neue Anforderungen wurden vorgestellt
    • Über 200 Anforderungen!
  • Technische Machbarkeit wurde im Nachgang geprüft
  • Prioritäten wurden nach technischer Einschätzung vergeben

Lösung

Löschen und neu Machen?!


"Nein"

Löschen und neu Machen?

  • Nicht machbar 2-3 Entwickler für 6 Monate abzustellen
  • Neue Funktionen dürfen nicht so lange auf sich warten lassen
  • Neuer Code kann nicht auf Perfomance getestet werden
  • Big Bang Release?! Was passiert wenn?

Refaktoring des Projekts

Challenge Accepted

Metriken erfassen

  • NPathComplexity Anzahl der azyklischen Ausführungspfade
  • Cyclomatic Complexity Anzahl linear unabhängiger Pfade
    • Definiert auch die Mindestanzahl an Tests

Metriken erfassen - Beispiel


                    public function substractPositiveNumbers(int $a, int $b, int $c, int $d): int
                    {
                                                1 2 3 4 5
                        if ($a < 0) { $a = 0; } 0 1 0 0 0
                        if ($b < 0) { $b = 0; } 0 0 1 0 0
                        if ($c < 0) { $c = 0; } 0 0 0 1 0
                        if ($d < 0) { $d = 0; } 0 0 0 0 1

                        return $a - $b - $c - $d;
                    }
                
  • Cyclomatic Complexity of 5

Metriken erfassen - Beispiel


                    public function substractPositiveNumbers(): int
                    {
                                                            1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6
                        if ($this->a < 0) { $this->a = 0; } 0 1 1 1 1 1 1 1 0 0 0 0 0 0 0 0
                        if ($this->b < 0) { $this->b = 0; } 0 0 1 0 0 0 1 1 1 0 0 0 0 1 1 1
                        if ($this->c < 0) { $this->c = 0; } 0 0 0 1 0 1 1 1 0 1 0 1 0 1 0 1
                        if ($this->d < 0) { $this->d = 0; } 0 0 0 0 1 1 0 1 0 0 1 1 1 0 1 1
                        return $this->a - $this->b - $this->c - $this->d;
                    }
                
  • Cyclomatic Complexity of 5
  • NPath complexity of 16

Metriken erfassen - Beispiel


                    public function substractPositiveNumbers(int $a, int $b, int $c, int $d): int
                    {
                        if ($this->a < 0) { $this->a = 0; }
                        if ($this->b < 0) { $this->b = 0; }
                        if ($this->c < 0) { $this->c = 0; }
                        if ($this->d < 0) { $this->d = 0; }
                        return $this->a - $this->b - $this->c - $this->d;
                    }

                    public function doWithA(): void
                    {
                        if ($this->a === 0) { $this->blackMagic(); }
                    }
                
  • Komplexität von?

Problemstellen identifzieren

  • The class SearchResultHandler has 3900 lines of code.
  • The method __construct has 20 parameters
  • The method getResultListClusters() has an NPath complexity of 3420 and a Cyclomatic Complexity of 20.
  • The method getSqlOfferFilter() has an NPath complexity of 14336 and a Cyclomatic Complexity of 16.
  • The method getOfferSummaries() has an NPath complexity of 15552 and a Cyclomatic Complexity of 19.
  • The method getSqlHotelFilter() has an NPath complexity of 98304 and a Cyclomatic Complexity of 19.

Testbarkeit Herstellen

  • Akzeptanztests
    • Abgleich der Response
  • Unittests
    • Extrahierte Klassen und Methoden testen
  • Perfomancetests
    • Monitoring der Anwendung
    • Nutzung von Featureflags

Perfomance Reports erstellen

  • N+1 Queries beim Rendern der Ergebnisse
  • Kartenansicht berechnet nicht benötigte Filter
  • Kartenansicht rendert alle Daten, gibt aber nur einen kleinen Teil aus

Trennung der Logik

  • Aufteilung in schlankere und logisch getrennte Endpunkte
  • Legacy Routen via Middleware abdecken


  • getSqlOfferFilter (NPath: 14336) -> Benötigt für Detail Filter
  • getSqlHotelFilter (NPath: 98304) -> Benötigt für Listen Filter

Extrahieren der Methoden

  • Abhängigkeiten reduzieren
  • Dadurch einfachere Testbarkeit
  • Weniger Seiteneffekte
  • Komplexität einzelner Methoden reduzieren

Code duplizieren

  • Featureflags nutzen
  • Einzelne Logiken gezielt umleiten
  • Bestandscode bleibt erhalten und funktioniert weiterhin

Beispiel Refactoring


    // before
    class SearchResulthandler {
        private function getSqlHotelFilter(ResultsRequest $request, array &$params): string
        {
            $sql = '';
            $this->distanceSetByFilter = false; // Interner State

            // filter by hotel id
            if ($request->getHotel_id()) {
                $sql .= ' AND cstc.cluster_id = :clusterId'; // SQL String ohne Kontext
                $params['clusterId'] = $request->getHotel_id();
            }
            // [...]
            return $sql;
        }
    }
                

Beispiel Refactoring


    class ResultsFilter { // Eigene Klasse
        private function addResultFilter(ResultsRequest $request, QueryBuilder $selectQuery): void
        {
            if ($request->getHotel_id()) {
                $selectQuery->andWhere('cluster_id = :clusterId'); // Query Builder
                $selectQuery->setParameter('clusterId', $request->getHotel_id(), Type::INTEGER);
            }
        }
    }
    class SearchResultHandler {
        private function getSqlHotelFilter(ResultsRequest $request, array &$params): string
        {
            $clusterIdSelect = $this->resultsFilter->getResultFilter($request);
            $params = array_merge($params, $clusterIdSelect->getParameters());
            // Minimal Eingriff in die Legacy Codebase
            return ' cluster_id IN ( ' . $clusterIdSelect->getSQL() . ' )';
        }
    }
                

Neue Funktionen implementieren

„Der Stornierungsfilter muss sich unterschiedlich verhalten“

                        // Before
                        class ResultsRequest {
                            public function getCancelableBitmask(): ?int;
                        }
                        class CompactResultsRequest extends ResultsRequest {
                        }

                        class SearchResultHandler {
                            function getSqlHotelFilter($sql, &$params): string {
                                // Check every possible Request type
                                if ($this->request->getCancelableBitmask()) {
                                    if ($this->request instanceof ResultsRequest) {
                                        $sql .= ' AND cancelable & :cancelableBitmask > 0';
                                        $params['cancelableBitmask'] = $this->request->getCancelableBitmask();
                                    }
                                    if ($this->request instanceof CompactResultsRequest) {
                                        $sql .= ' AND cancelable & :cancelableBitmask = :cancelableBitmask';
                                        $params['cancelableBitmask'] = $this->request->getCancelableBitmask();
                                    }
                                }
                                return $sql;
                            }
                        }
                    

Neue Funktionen implementieren

„Der Stornierungsfilter muss sich unterschiedlich verhalten“

                        // After
                        interface ResultsClusterOfferFilterableInterface {
                            public function getCancelableBitmask(): ?int;
                        }
                        interface OfferFilterableInterface {
                            public function getCancelableBitmask(): ?int;
                        }

                        class ResultsOfferClusterFilter {
                            function addResultFilter(QueryBuilder $selectQuery, ResultsRequest $request): void {
                                if ($request->getCancelableBitmask()) {
                                    $selectQuery->andWhere('cancelable & :cancelableBitmask > 0');
                                    $selectQuery->setParameter('cancelableBitmask', $request->getCancelableBitmask(), Type::INTEGER);
                                }
                            }
                        }

                        class OffersFilter {
                            function addResultFilter(QueryBuilder $selectQuery, ResultsRequest $request): void {
                                if ($request->getCancelableBitmask()) {
                                    $selectQuery->andWhere('cancelable & :cancelableBitmask = :cancelableBitmask');
                                    $selectQuery->setParameter('cancelableBitmask', $request->getCancelableBitmask(), Type::INTEGER);
                                }
                            }
                        }
                    

Vergleich der Metriken

PHPMetrics Analyse

Vorher

Nachher

Übersicht

Vorher Nachher
Average cyclomatic complexity by class 23.31 11.84
Lines of Code 5823 7922
Classes 35 91

Perfomance Monitoring

Beispielwerte 05.05 01.09
# Requests Listenansicht ~ 300k
AVG Response Time Listenansicht 316ms 263ms
# Requests Detailansicht ~ 610k
AVG Response Time Detailansicht 168ms 155ms
# Requests Vertriebspartner ~ 900k
AVG Response Vertriebspartner 110ms  59ms
# Requests Kartenansicht ~ 250k
AVG Response Kartenansicht 264ms 343ms

Lessons Learned

Motivierend

  • Relative Testabdeckung steigt deutlich
  • Angst für Erweiterungen entfällt
  • Es macht wieder "Spaß"

Überblick bekommen

  • Aufgaben der Services identifzieren und aufteilen
  • Prioritäten festlegen zwischen Umbau und Features
  • Testplan erstellen
  • Prozesse visualisieren

Umbau nicht liegen lassen

  • Duplizierter Code weiterhin vorhanden
    • Auf Dauer ist es komplexer
  • Metriken zeigen größere Komplexität

Kleine Schritte gehen

  1. Code Duplizieren
  2. Anwendung Testen
  3. Tests ergänzen
  4. Methode refaktorieren
  5. Internen State vermeiden
  6. Funktionen ergänzen
  7. Code Aufräumen

Wartet nicht zu lange