Code Smells

[Coffeecoder]

Hallo DP-Mitglieder,

Ich habe ein kleinen Kurs über das Thema "Code Smells" für euch vorbereitet.
Das Ziel dieses Kurses ist das Verstehen was "Code Smells" sind. Denn jeder Programmierer hat (unbewusst) solche in seinem Code.

Theorie:

Das Wort "Code Smells" was übersetzt "(schlechter) Geruch" heißt,
stammt von Kent Beck und wurde durch das Buch Refactoring von Martin Fowler verbreitet.

Code Smells ist ein schlecht strukturierter Programmcode.
Der Code ist für den oder einen anderen Programmierer schwer zu verstehen.

Bei Verbesserungen oder Erweiterungen kommen neue Fehler dazu.
"Code Smells" wird erst erkannt bei einer Überarbeitung des Programmcodes.

Welche "Code Smells" existieren?

Es gibt eine Unmenge von schlecht-struktierter Programmcode-Beispiele.
Hier einige Beispiele die sich auf die objektorientierte Programmierung (Java, Delphi, ...) beziehen:

1. Code-Vermehrung: Der identische Code taucht im Programm an mehreren Stellen auf.
2. Langer Code in Methode: Der Programmcode in einer Funktion, Prozedur ist zu lang.
3. Lange Parameterliste: einige übergebene Parameter die unnötig sind.
4. Nichtaussagender Name: Aussagekräftige Name erfüllen das Verständnis des Codes oder Inhalt einer Variable.
5. Toter Code: Ein Programmcode welches nicht mehr gebraucht wird.

Quelle : http://de.wikipedia.org/wiki/Code_smells

Natürlich gibt es noch mehrere Arten und interessante Bücher die das Thema detaillierter
beschreiben wie z.B das Buch : "Clean Code - Refactoring, Patterns, Testen und Techniken für sauberen Code" von Robert C. Martin.

Beispiele/Tipps:

1. "Code-Smell"
   
   Delphi-Quellcode:

   procedure anpassenChart;
   begin
                   with Chart1 do
                   begin
                          Legend.visible := false;
                          View3D := false;
                          Title.Text.Clear;
                          Title.Text.Add('Chart');
                   end;
   
                   with Chart2 do
                   begin
                          Legend.visible := false;
                          View3D := false;
                          Title.Text.Clear;
                          Title.Text.Add('Chart');
                   end;
   
                   with Chart3 do
                   begin
                          Legend.visible := false;
                          View3D := false;
                          Title.Text.Clear;
                          Title.Text.Add('Chart');
                   end;
   end;

   Es ist nicht falsch, der Code funktionniert ja. Aber was ist schlecht daran?
   Ganz einfach: Es sollen jetzt doch Legende visible sein. Was macht man?
   Legend.visible := true setzen und Copy-Paste.
   Das ist anfällig für Fehler, einmal nicht aufgepasst und schon ein Legend.visible := false vergessen zu überschreiben.
   
   So macht man es besser:
   
   Delphi-Quellcode:

   procedure anpassenChart(ch : TChart);
   Begin
                   with ch do
                   begin
                         Legend.visible := false;
                         View3D := false;
                         Title.Text.Clear;
                         Title.Text.Add('Chart');
                   end;
   end;

   Aufruf:
   
   Delphi-Quellcode:

   ...
   var i : integer;
   begin
           for  i := 1 to 3 do anpassenChart('Chart'+IntToStr(i));
   end;

2. und
3. Beide Punkten sind verwandt, denn es geht ja um Methoden (Funktion, Prozedur).
   Hier zeige ich kein Beispiel, aber ein paar Tipps:
   1. Kurze Parameterliste, welche für eine einzige Aufgabe gebraucht werden.
   2. Es sollen nicht mehr als 20 Programmzeilen in einer Methode stehen.
   3. Eine Methode soll nur eine Aufgabe erfüllen. (eine Funktion die den Flächeninhalt eines Kreises berechnet, soll nicht noch den Umfang berechnen, dies sollte in einer separaten Funktion untergebracht werden.)
4. Ich komme sofort zu einem Beispiel:
   
   Schlechter Name:
   
   
   Delphi-Quellcode:

   var
        d : integer;
   begin
          ...
          if d  ... 
   end;

   Was will die Variable d uns sagen? Nicht viel!
   
   Besser:
   
   
   Delphi-Quellcode:

   ...
   var
        TageSeitErstellung : integer;
   
   begin
         ...
         if TageSeitErstellung ....
   
   end;

   Hier sagt die Variable TageSeitErstellung uns der Zweck dieser Variable aus. Man weiss sofort, der Inhalt ist die Anzahl Tage seit der Erstellung von ...
   
   Noch ein typisches Beispiel mit dem Typ boolean:
   
   
   Delphi-Quellcode:

   var
         Daten : boolean;

   Daten sagt wieder nichts aus. Man kann es so verstehen:
   1. DatenGeLoescht
   2. DatenVorhanden
   3. DatenGesendet
   4. DatenVerarbeitet
   5. DatenKorrupt
   6. ...
   
   
5. Toter Code ist :
   1. Ein auskommentierter Code
   2. Ein Codestück welches immer übersprungen wird. Hier ist die if-Anweisung überflüssig:
      
      Delphi-Quellcode:

      var
        N : int;
      
      begin
              N := 0;
              ...
              if N = 3 then
              begin
                   ....
              end;
              ...
      end;

Schlusswort:

Kennt man einige Code-Smells, so kann man diese vermeiden, damit der Programmcode nicht "schlecht riecht".

Ich hoffe ich könnte euch ein bisschen über dieses interessante Thema erzählen.
Bitte um konstruktive Kritik.

Danke bis Bald.

Mfg Coffeecoder

[Coffeecoder]

Zitat von bernau:

Du wirst lachen, aber lokale Variablen fangen bei mir tatsächlich mit (einem kleinen) L an. Wobei das nich ganz optimal ist, da man das "l" mit einer "1" (in Worten "Eins") verwechseln kann. Gibt auch einige Styleguides die das bemängeln. Da bei mir aber keine Variable mit einer Ziffer anfängt, kann ich das verschmerzen.

Ich dachte, du kannst keine Ziffer als Präfix vor einem Variablennamen schreiben
Dies ist doch nicht erlaubt resp der Compiler meckert doch : 1int;

[himitsu]

Das Einzige, was ich total bescheuert finde, das ist der string.

Ist erstmal eigenartig, daß diese (Basis)Typen kein T davor haben und dann das fette String.


Auf Arbeit schreibe ich es (da es überall so ist) klein, aber privat lieber groß.
Und warum schreibt man groß klein, aber das Kleine groß?
Blau

[DeddyH]

Wenn keine Verwechslungsgefahr (z.B. mit Properties) besteht, dann lasse ich Präfixe bei Argumenten auch am liebsten weg, ansonsten sollte der Weg mit dem vorangestellten A meistens funktionieren (es sei denn, es besteht wieder Verwechslungsgefahr). Ganz blöde wird es bei solchen Konstrukten (das with ist in diesem Zusammenhang natürlich Absicht):

Delphi-Quellcode:

constructor TMeineKomponente.Create(AOwner: TComponent; Name: string);
begin
  FDings := TDings.Create;
  with FDings do
    begin
      Name := Name;
    end;

[bernau]

Zitat von Coffeecoder:

Zitat von bernau:

Du wirst lachen, aber lokale Variablen fangen bei mir tatsächlich mit (einem kleinen) L an. Wobei das nich ganz optimal ist, da man das "l" mit einer "1" (in Worten "Eins") verwechseln kann. Gibt auch einige Styleguides die das bemängeln. Da bei mir aber keine Variable mit einer Ziffer anfängt, kann ich das verschmerzen.

Ich dachte, du kannst keine Ziffer als Präfix vor einem Variablennamen schreiben
Dies ist doch nicht erlaubt resp der Compiler meckert doch : 1int;

Deswegen fängt bei mir ja auch keine Variable mit einer Ziffer an

[himitsu]

Syntax für Alles ist [{Alpha}_][{AlphaNum}_]* aber eigentlich isses [{Letter}_][{Letter}{Digit}_]* und in ANSI-Delphis [a-zA-Z_][a-zA-Z0-9_]* bei Unitnamen und Componentennamen (TComponent.Name) [{Alpha}_][{AlphaNum}_.]* (früher waren keine Punkte erlaubt)
Aber dennoch besser keine mehrfach zusammenhängenden Punkte und keine Punkte am Ende.

Ab Delphi 2009 darf ich also auch Müssen, Muß oder 秘密 als Name verwenden.

Einige verdammen es, aber ich find's geil.