AGB  ·  Datenschutz  ·  Impressum  







Anmelden
Nützliche Links
Registrieren

[CleanCode] Beispielklasse TDataLocation

Ein Thema von neo4a · begonnen am 18. Feb 2012 · letzter Beitrag vom 20. Feb 2012
Antwort Antwort
Seite 2 von 4     12 34   
Furtbichler
(Gast)

n/a Beiträge
 
#11

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 12:58
Müsste auf dasselbe herauskommen.
Sehe ich auch so: Es macht aus 7 Zeilen 1, ohne dass die Lesbarkeit wirklich leidet.
Ich nicht, weil:
1. Die Auswertereihenfolge ist compilerabhängig.
2. Die Funktion ist abhängig vom Compilerschalter ('Complete boolean evaluation')

Absolute nicht 'clean code' tauglich.

Geändert von Furtbichler (18. Feb 2012 um 13:01 Uhr)
  Mit Zitat antworten Zitat
neo4a

Registriert seit: 22. Jan 2007
Ort: Ingolstadt
362 Beiträge
 
Delphi XE2 Architect
 
#12

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 13:14
Müsste auf dasselbe herauskommen.
Sehe ich auch so: Es macht aus 7 Zeilen 1, ohne dass die Lesbarkeit wirklich leidet.
Ich nicht, weil:
1. Die Auswertereihenfolge ist compilerabhängig.
2. Die Funktion ist abhängig vom Compilerschalter ('Complete boolean evaluation')

Absolute nicht 'clean code' tauglich.
Ich bin hier kein "CleanCode-Schiedsrichter", aber ich stimme Dir zu und habe das Listing entsprechend angepasst. Deine Lösung lässt sich auch etwas besser Debuggen/Nachverfolgen. Einzig der exit-Sprung zur Zuweisung der out-Variablen im finally-Block behagt mir nicht so ganz.
Andreas
  Mit Zitat antworten Zitat
Furtbichler
(Gast)

n/a Beiträge
 
#13

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 13:37
Hi, ich sage bereits, das die Alternative mit 'exit' kompakter, aber vielleicht nicht lesbarer ist (worum es beim CC ja auch geht).

Der Grund ist:
Deine Lösung beschreibt den Vorgang direkt ('Wenn noch nicht erfolgreich, versuche die nächste Möglichkeit').
Bei meiner Lösung muss man wissen, das ein "exit" (dagegen ist an sich nichts einzuwenden) in den finally-Abschnitt springt. Die Aussage ist 'Probiers aus und wenn es geklappt hat, fertig'.

Man könnte auch mit Exceptions arbeiten ('Probiere nächsten Schritt und wenns klappt, Abbruch').
  Mit Zitat antworten Zitat
Benutzerbild von himitsu
himitsu

Registriert seit: 11. Okt 2003
Ort: Elbflorenz
40.504 Beiträge
 
Delphi 11 Alexandria
 
#14

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 13:44
Man könnte auch mit Exceptions arbeiten ('Probiere nächsten Schritt und wenns klappt, Abbruch').
Ein Exit innerhalb eines Try-Finally löst eine stille Exception aus, um in das Finally zu springen.

Aber so im Allgemeinen (abgesehn von diese impliziten Exit-Exception und von EAbort) sind Exceptions/Ausnahmen nicht für eine explizite Programmsteuerung vorgesehn. Und ich bin mir fast sicher, daß selbst CleanCode was dagegen hätte.


Wer FullBooleanEval global aktiviert, ist selber Schuld und muß mit den Konsequenzen leben.
Wenn das wirklich mal benötigt würde, dann sollte man es nur lokal aktivieren.

Aber wer als Entwickler sich absichern will, kann sowas ja nochmal explizit angeben, am Anfang seiner Dateien, bzw. in einer globalen Include-Datei.
Garbage Collector ... Delphianer erzeugen keinen Müll, also brauchen sie auch keinen Müllsucher.
my Delphi wish list

Geändert von himitsu (18. Feb 2012 um 13:50 Uhr)
  Mit Zitat antworten Zitat
Benutzerbild von Uwe Raabe
Uwe Raabe

Registriert seit: 20. Jan 2006
Ort: Lübbecke
9.846 Beiträge
 
Delphi 11 Alexandria
 
#15

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 14:03
1. Die Auswertereihenfolge ist compilerabhängig.
2. Die Funktion ist abhängig vom Compilerschalter ('Complete boolean evaluation')

Absolute nicht 'clean code' tauglich.
Ich kann das Clean Code Buch jetzt nicht auswendig, aber meines Wissens steht da weder was über "Clean Code ist portabel", noch "Clean Code ist resistent gegen Umgebungsänderungen". Im Zweifelsfall prüft man das mit {$IFOPT B+} und wirft einen Compilerfehler.

Was die Auswertereihenfolge betrifft: Ein Compiler, der ein Konstrukt wie if (P<> nil) and P.Enabled then in eine Schutzverletzung führt, hat bei m.E. sowieso keine Chancen.
Uwe Raabe
Certified Delphi Master Developer
Embarcadero MVP
Blog: The Art of Delphi Programming
  Mit Zitat antworten Zitat
Benutzerbild von himitsu
himitsu

Registriert seit: 11. Okt 2003
Ort: Elbflorenz
40.504 Beiträge
 
Delphi 11 Alexandria
 
#16

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 14:21
if Assinged(P) then if P.Enabled then sieht och nicht besser aus, aber wenn Clean-Code sowas verlangt, dann bin ich für Dirty-Code.
Garbage Collector ... Delphianer erzeugen keinen Müll, also brauchen sie auch keinen Müllsucher.
my Delphi wish list
  Mit Zitat antworten Zitat
Furtbichler
(Gast)

n/a Beiträge
 
#17

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 14:36
Eine Diskussion über den Einsatz von 'exit', und ob ein 'exit' eine Ausnahme darstellt (oder nur intern so compiliert wird), hatten wir schon oft und driftet in Richtung 'Religion' ab.

Eine 'Ausnahme' im Programmfluss ist ja nichts anderes als das Gegenteil der Regel. Es ist sehr wohl "sauberer Code", mit Ausnahmen zu arbeiten.
Der Grund ist recht einfach: Durch die Verwendung von Exceptions kann ich den normalen Programmfluss viel direkter beschreiben.

Hier könnte man durchaus -ähnlich dem 'exit' Konstrukt- mit Exceptions (na, vielleicht EAbort) arbeiten. Ich würde die aufgerufenen 'FindXXX'-Methoden umbenennen, damit klar wird, das etwas versucht wird. Die Methoden liefern dann kein Bool, sondern werfen eine EAbort-Exception.

Delphi-Quellcode:
function TDataLocation.ConfigFile(out aConfigFileName : string; const aDefaultFileName : string = '') : Boolean;
begin
    if aDefaultFileName > 'then
      FDataFile := aDefaultFileName;
    try
      Result := True;
      TryToFindDataCmd;
      TryToFindConfigCmd;
      TryToFindLocalConfig;
      TryToFindLocalFile;
      TryToFindUserDataFile;
      Result := False;
   finally
      aConfigFileName := FDataFile;
   end;
end;
Was mir dann aber nicht gefällt, ist die unterschiedliche Verwendung der Behandlung eines Methodenresultats. Es existieren ja (mindestens) zwei Ansätze:
* Implementieren als Funktion und zurückliefern eines Resultats
* Implementieren als Methode und werfen einer Exception

Clean Code favorisiert hier eindeutig die Verwendung von Exceptions.

Nun, wenn man in seiner Klasse / Framework schon auf Funktionsresultate à la 'hat geklappt / hat nicht geklappt' setzt, sollte man das auch so durchziehen.

Das Programmieren mit dem Ziel des 'Clean Code' ist ein ständiges Abwiegen von Varianten. Es muss sich 'gut anfühlen', und das ist in erster Linie ein subjektives Gefühl. Das heißt natürlich nicht, das CC nur aus dem guten Gefühl darstellt, es müssen auch Grundregeln eingehalten werden.

Merke: 'Clean Code' ist nur eine Bewegung, die sich Regeln auferlegt, um immer besseren (im Sinne der Regeln) Code zu produzieren.

..aber meines Wissens steht da weder was über "Clean Code ist portabel", noch "Clean Code ist resistent gegen Umgebungsänderungen".
Nein, aber da steht auch 'der Code muss lesbar und selbsterklärend sein'. Das mit dem 'portabel' versteht sich von selbst. Denn 'portabel' ist gleichbedeutend mit 'robust' und 'wartbar'.

Was die Auswertereihenfolge betrifft: Ein Compiler, der ein Konstrukt wie if (P<> nil) and P.Enabled then in eine Schutzverletzung führt, hat bei m.E. sowieso keine Chancen.
Stimmt, aber das steht da ja nicht. Es handelt sich um funktional unabhängigen Code ('FindXXX'), die ein gewitzter Compiler durchaus parallel oder anhand von Heuristiken in anderer Reihenfolge abarbeiten könnte.
Des Weiteren vergisst Du leider vollkommen, das die Auswertereihenfolge nur bei unabhängigen Termen machbar ist und jeder, aber auch wirklich jeder, der ein Semester Compilerbau studiert hat, bei so eine Zwischenbemerkung schallendes Gelächter geerntet hätte.

Wer FullBooleanEval global aktiviert, ist selber Schuld und muß mit den Konsequenzen leben.
Jupp. Aber wenn es aus Versehen passiert ist, sollte es keine Auswirkungen auf die Funktionalität haben, gell?

Zitat:
Aber wer als Entwickler sich absichern will, kann sowas ja nochmal explizit angeben, am Anfang seiner Dateien, bzw. in einer globalen Include-Datei.
Du hast den Clean-Code Gedanken nicht verstanden.

if Assinged(P) then if P.Enabled then sieht och nicht besser aus, aber wenn Clean-Code sowas verlangt, dann bin ich für Dirty-Code.
Das merkt man bei deiner Art, zu programmieren Aber nebenbei: CC verlangt keine Hosenträger, wo die Hose eng sitzt und ein Gürtel verwendet wird. Wenn Du deinen Code so schreibst, das Eingabeparameter abgefragt werden, kannst Du darauf verzichten.

Ich sags nochmal: CC hat nichts mit kompakt zu tun oder mit performant. Sondern nur mit lesbar, robust, testbar, wartbar.

Geändert von Furtbichler (18. Feb 2012 um 14:40 Uhr)
  Mit Zitat antworten Zitat
Furtbichler
(Gast)

n/a Beiträge
 
#18

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 15:06
Zurück zum Thema:
Die Funktion "GetCmdLineSwitchValue" verstößt mindestens gegen die CC-Prinzipien "Command Query Separation" sowie "Do one thing": Die Funktion ermittelt den Wert eines Kommandozeilenparameters UND liefert TRUE bei Erfolg.

Ich würde es genauso umsetzen, aber 'sauber' im Sinne von CC wäre das nicht. Es ist aus dem Funktionsnamen nicht ersichtlich, das "aFilename" geliefert wird; man muss 2x hinschauen. Da es keine zeitliche Koppelung zwischen 'Vorhandensein des Parameters' und 'Extrahieren des Parameters' gibt, sollte man die Funktion aufteilen (nur im Sinne von CC)

Delphi-Quellcode:
...
   if GetCmdLineSwitchValue(aFileName, FConfigCmdParam) then
...
CC liefert hier ein Beispiel, wie es besser geht:

Delphi-Quellcode:
If CmdLineSwitchExists(FConfigCmdParam) then
  aFilename := GetCmdLineSwichValue(FConfigCmdParam);
  Mit Zitat antworten Zitat
Benutzerbild von himitsu
himitsu

Registriert seit: 11. Okt 2003
Ort: Elbflorenz
40.504 Beiträge
 
Delphi 11 Alexandria
 
#19

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 15:12
Jupp. Aber wenn es aus Versehen passiert ist, sollte es keine Auswirkungen auf die Funktionalität haben, gell?
Grob übersetzt würde CleanCode dann aber bedeuten auf nahezu alle Codeoptimierungen zu verzichten.

Ein gewisses Standard-Verhalten würde ich einfach voraussetzen
und wenn dieses nicht zutrifft, dann darf auch keiner ein standardmäßig korrektes Verhalten voraussetzen.

Denn mit voller booleanischen Auswertung kann der Code nur in if Assinged(DS) then if DS.Active then if Assinged(DS.FindField('x')) then if not DS.FieldByName('x').IsNull then if DS.FieldByName('x').AsString = 'testthen enden.
Selbst mit Zeilenumbrüchen und Einrückung wird es nicht lesbarer.
Garbage Collector ... Delphianer erzeugen keinen Müll, also brauchen sie auch keinen Müllsucher.
my Delphi wish list
  Mit Zitat antworten Zitat
neo4a

Registriert seit: 22. Jan 2007
Ort: Ingolstadt
362 Beiträge
 
Delphi XE2 Architect
 
#20

AW: [CleanCode] Beispielklasse TDataLocation

  Alt 18. Feb 2012, 15:49
Die Funktion "GetCmdLineSwitchValue" verstößt mindestens gegen die CC-Prinzipien "Command Query Separation" sowie "Do one thing": Die Funktion ermittelt den Wert eines Kommandozeilenparameters UND liefert TRUE bei Erfolg.

Ich würde es genauso umsetzen, aber 'sauber' im Sinne von CC wäre das nicht. Es ist aus dem Funktionsnamen nicht ersichtlich, das "aFilename" geliefert wird; man muss 2x hinschauen.
Dieser Einwand trifft ja auch zu auf die Haupt-Funktion:
function ConfigFile(out aConfigFileName : string; const aDefaultFileName : string = '') : Boolean; Aber wie Du schon vorher gesagt hast, ist CC keine Religion, sondern eher "professioneller Pragmatismus". Nun ist die Ermittlung von Kommandozeilen-Parameter kein besonders "teurer" Vorgang, so dass sie sicherlich auch 2x durchlaufen werden könnte. Ich würde es trotzdem nicht machen und lieber an der aufrufenden Stelle einen Kommentar hinterlassen.
Andreas
  Mit Zitat antworten Zitat
Themen-Optionen Thema durchsuchen
Thema durchsuchen:

Erweiterte Suche
Ansicht

Forumregeln

Es ist dir nicht erlaubt, neue Themen zu verfassen.
Es ist dir nicht erlaubt, auf Beiträge zu antworten.
Es ist dir nicht erlaubt, Anhänge hochzuladen.
Es ist dir nicht erlaubt, deine Beiträge zu bearbeiten.

BB-Code ist an.
Smileys sind an.
[IMG] Code ist an.
HTML-Code ist aus.
Trackbacks are an
Pingbacks are an
Refbacks are aus

Gehe zu:

Impressum · AGB · Datenschutz · Nach oben
Alle Zeitangaben in WEZ +1. Es ist jetzt 07:05 Uhr.
Powered by vBulletin® Copyright ©2000 - 2022, Jelsoft Enterprises Ltd.
LinkBacks Enabled by vBSEO © 2011, Crawlability, Inc.
Delphi-PRAXiS (c) 2002 - 2021 by Daniel R. Wolf