Sehr wahrscheinlich hat jeder, der sich beruflich mit Software-Entwicklung befasst, schon einmal fremden Code übernehmen und weiterentwickeln und/oder darin Fehler beheben müssen. Und die Chance ist gross, dass der übernommene Code nicht dem entsprochen hat, was man von gut wartbarem Code erwartet.

Hier ist meine persönliche und subjektive Liste der unbeliebtesten Entdeckungen in fremden Code. Mit dieser Liste will ich niemanden angreifen oder blossstellen. Und auch ich habe schon die eine oder andere Sünde begangen, die hier aufgelistet ist. Ich möchte mich deshalb an dieser Stelle bei allen meinen Nachfolgern entschuldigen, die solchen Code von mir geerbt haben.

Hier nun aber die Liste:

9. Riesige Dateien
   Wenn eine Datei mehrere Tausend Zeilen Code enthält wurde zu viel in diese Datei gepackt. Wenn die Datei dann nur eine Klasse enthält ist entsprechend auch die Klasse zu gross. Meist enthält die Klasse dann zu viel Know-How, d.H. sie behandelt mehrere Aspekte, anstatt sich auf ihre Kernkompetenz zu konzentrieren.
   Ob die Datei nun eine oder mehrere Klassen enthält, das Resultat ist schlussendlich das selbe: die Übersicht geht verloren und das Verhalten ist dementsprechend schwierig nachzuvollziehen.
   Generierter Code ist von dieser Regel ausgenommen.
8. Mehrere Klassen in einer Datei
   Rang 9 ist manchmal das Resultat von diesem Vergehen. Die beiden können aber auch getrennt auftreten.
   Ich gehe gerne, wenn mir eine Klasse mehrmals aufgefallen ist, über den Solution Explorer in Visual Studio zu der entsprechenden Klasse. Wenn die gesuchte Klasse aber zusammen mit anderen Klassen in einer Datei liegt ist die Chance gross, dass die Datei nicht den passenden Namen hat und die Klasse deshalb nicht (einfach) auffindbar ist.
   Generierter Code ist, wie schon bei Rang 9, von dieser Regel ausgenommen.
7. Viele Warnungen
   Warnungen sind da, um mögliche Probleme aufzuzeigen. Das können z.B. unbenutzte Variablen oder nicht erreichbare Code-Abschnitte sein, aber auch veraltete (obsolete) Funktionen oder fehlende XML-Kommentare. Einzelne Warnungen können normalerweise sehr einfach behoben oder kurzfristig auch mal ignoriert werden, wenn z.B. dieser Codeabschnitt gerade überarbeitet wird. Wenn die Warnung langfristig ignoriert werden soll kann dies mit einer Compilerdirektive (pragma) angegeben werden.
   Wenn die Warnungen aber im Rudel auftreten kann nicht mehr unterschieden werden, welche Warnung nun wichtig und welche unwichtig ist bzw. welche Warnung schon seit langem auftritt und welche neu hinzugekommen ist. Wenn man Code vom Vorgänger übernommen hat kann man auch nicht abschätzen, welche Warnungen ein mögliches Problem aufzeigen und welche Warnungen ignoriert werden können.
6. Irreführende Formatierungen
   Wahrscheinlich ist schon jedem ein Fundstück wie das folgende unter die Augen gekommen:

   1 2 3 4

   if(doCheck())   doSomething();   doAnotherThing(); doAThirdThing();

   Auf den ersten Blick könnte man anhand der Formatierung meinen, dass die if-Abfrage die nächsten beiden Funktionen beeinflusst. Dem Compiler ist aber die Formatierung meistens (Python ist da z.B. eine Ausnahme) egal. Viel Spass beim Fehler suchen!

5. Undokumentierte Workarounds
   Manchmal muss man weniger schöne Lösungen benutzen, um Fehler in 3rd Party Libraries zu umgehen. Oder man findet einfach keine bessere Lösung und die Deadline rückt näher. Für diese Situationen habe ich Verständnis. Aber bitte nicht irgendwo im Code versteckt, so dass man beim Fehler suchen an dieser Stelle sucht, da der Code hier so „schräg“ aussieht (Dieser Code sieht so falsch aus, der Fehler muss hier liegen…).
   Wenn ein Workaround eingesetzt werden muss darf der Kommentar nicht fehlen, dass es ein Workaround ist. Und aus welchem Grund er nötig ist. Im Idealfall mit einem Verweis auf den Bugreport im Bugtracker der 3rd Party Library. So kann der Workaround entfernt werden, wenn eine neue Version der Library verwendet wird, in welcher der Bug behoben ist und der Workaround nicht mehr nötig ist.
4. Versteckte „globale“ Variablen
   Code-Fundstücke wie das folgende liebe ich:

   1 2 3 4 5

   if(doCheck()) {     DataBaseAccess dba = new DataBaseAccess();     dba.CurrentDB = "myDataBase"; }

   Mein erster Impuls ist hier: „Das Objekt dba fällt gleich wieder aus dem Scope, dieser ganze Block kann entfernt werden.“
   Beim Untersuchen der Klasse DataBaseAccess wird dann die Aufgabe dieses Code-Blockes sichtbar:

   1 2 3 4 5 6 7 8 9 10

   public class DataBaseAccess {     private static string currentDB = "";     public string CurrentDB     {         get { return currentDB; }         set { currentDB = value; }     } }

   DataBaseAccess hat eine static Membervariable, auf die mit einem non-static Property zugegriffen wird. Obwohl das Objekt aus dem Scope fällt bleibt deshalb der Wert weiterhin gesetzt. Für einen Anwender der Klasse sind anhand der Schnittstelle weder die Funktionalität des ersten Code-Blocks noch die damit verbundenen Seiteneffekte ersichtlich. Wenn schon static Membervariablen verwendet werden müssen sollte auch der Zugriff auf diese über static Properties erfolgen.

3. Irreführende Methoden- und Properties-Bezeichnungen
   Wenn z.B. eine Methode den Namen SetReportMode() hat, dann aber noch fünf andere Dinge gleichzeitig erledigt – oder im schlimmsten Fall nur die fünf anderen Dinge erledigt aber den Report Mode gar nicht einschaltet – sind die Probleme im wahrsten Sinn des Wortes vorprogrammiert.
2. Business-Logik (nur) im User Interface
   Da wurde ein schönes Schichtenmodell aufgestellt, aber die Business-Logik befindet sich zur Hälfte oder komplett im User Interface Layer und nicht im Business Layer. Zum einen sucht man dadurch Fehler oder die Ansatzpunkte für Erweiterungen am „falschen“ Ort, zum andern kann auch das User Interface nicht einfach gewechselt werden (z.B. Windows Forms durch WPF oder ASP.NET ersetzen), wie es mit einem sauberen Schichtenmodell möglich wäre. Wenn die Business Logik dann noch verteilt ist dürften auch viele Business Regeln mehrfach implementiert sein, vermutlich sogar mit kleinen Unterschieden (sprich Inkonsistenzen).
1. Keine Unit Tests
   Diese Entdeckung tritt meist in Gesellschaft von vorher aufgeführten Entdeckungen auf. In diese Kategorie gehört auch die Variante „als Unit Test getarnter Spike„, bei der erste Versuche an einer Technologie mit Hilfe des Test Runners gemacht werden. Auch andere „Missbräuche“ des Test Runners gehören in diese Kategorie. Diese zeichnen sich dadurch aus, dass die keine Asserts in den Testmethoden haben.
   Alle Varianten haben aber eines gemeinsam: Es fehlt die Absicherung, um Refactorings vornehmen zu können ohne Angst haben zu müssen, etwas kaputt zu machen. Und wenn die Unit Tests fehlen ist normalerweise der Code auch nicht so geschrieben, dass Unit Tests einfach hinzugefügt werden können. Dadurch muss der Code zuerst umgebaut werden, dabei fehlt aber wieder die Unterstützung der Unit Tests… Und um die vorher aufgeführten Entdeckungen beheben zu können wären die Unit Tests auch eine willkommene Absicherung.

Wie schon oben gesagt ist dies meine subjektive Top 9. Was sind Eure schlimmsten Entdeckungen in fremdem Code oder wie würde Eure Reihenfolge aussehen?