Clean Code Frage

[MyRealName]

Also, wir versuchen in der Firma Clean Code zu nutzen und habe dazu eine Frage, die ich so noch nicht beantwortet gefunden habe :

Nehmen wir an, ich habe ein Event eines TDataSet OnSetText. Darin stelle ich eine Überprüfung an, ob der eingegebene Wert zulässig ist und entweder nehme ich ihn dann an oder weise ihn ab. Das ganze ist 30 Zeilen von "procedure" bis zum "end". Nun sagt mein Code-Reviewer, dass dies in eine eigene Funktion muss, zum Bsp (vereinfacht) "IsValidValue". Aber das SetText selbst ist ja eh schon ein Validator-Event und der Aufruf einer funktion in diesem Event, was eh nur eine Sache macht, bringt mir keinen Mehrwert an Leserlichkeit.
Ich kann es natürlich ändern wie es der Reviewer möchte, aber es geht ja um das Grundsätzliche...

Gedanken dazu ?

[haentschman]

Hallöle...:P

Zitat:

Nun sagt mein Code-Reviewer, dass dies in eine eigene Funktion muss, zum Bsp (vereinfacht) "IsValidValue".

...so einen habe ich Gott sei Dank nicht. :P
Persönlich habe ich auch Prozeduren die thematisch nicht trennbar sind...weil sie nur für diese Funktion relevant sind. Damit das nicht Spagetticode wird, lagere ich den Code in lokale Prozeduren aus. Übersichtlicher und trotzdem an einem "Platz". :thumb:

Kann er damit leben?

PS: In allen z.B. GUI Events darf es nur den Aufruf der procdure geben. Innerhalb dieser "verzweigen"

[Uwe Raabe]

Zitat:

Zitat von MyRealName (Beitrag 1470446)

Gedanken dazu ?

Bitte schön: When, What, Why!

[TiGü]

Zitat:

...bringt mir keinen Mehrwert an Leserlichkeit.

Aber vielleicht in Sachen Testbarkeit, wenn das als eigene Methode implementiert ist?

[sakura]

Allein wegen der Testbarkeit gehört es da nicht hin.

Wenn Du diese Überprüfung doch mal an anderer Stelle benötigst, dann ist die jetzt schon gut versteckt.

Ein Überprüfung der Werte gehört in die Klasse (besser in den Service), welcher alle Aspekte dieses Datum bzw. der Datengruppe überprüft und kennt. Diese Klasse kann dann getestet werden und recht einfach (Dependency Injection) durch eine neue Klasse ausgetauscht werden, wenn nötig.

...:cat:...

[MyRealName]

Testbarkeit ist ein gutes und verständliches Argument, auch wenn wir bisher keine Unittests machen und unser grosser Chef das bisher auch noch nicht bewilligt hat.
Vielen Dank für die Ideen.

[sakura]

Zitat:

Zitat von MyRealName (Beitrag 1470461)

unser grosser Chef das bisher auch noch nicht bewilligt hat...

:pale:

Was muss da bewilligt werden? Und was spricht für ihn dagegen?

...:cat:...

[himitsu]

Aber wenn eh alles in der einen Unit steckt, dann macht es vom Test her doch garkeinen Unterschied, ob er die EventMethode oder die private Methode in der selben Klasse aufruft?

[Lemmy]

Zitat:

Zitat von sakura (Beitrag 1470465)

Zitat:

Zitat von MyRealName (Beitrag 1470461)

unser grosser Chef das bisher auch noch nicht bewilligt hat...

:pale:

Was muss da bewilligt werden? Und was spricht für ihn dagegen?

...:cat:...

Du solltest dich mal, nach einem Unit-Test Vortrag / Workshop auf der EKON, natürlich verkleidet, unter die Besucher mischen, das erweitert ungemein :-) Mein Highlight war mal nach einem Ua oder Nick Hodges Vortrag, quasi beim verlassen des Raumes: das ist doch alles akademisch, das kann man doch in einem "richtigen" Projekt gar nicht nutzen... Und sein Kollege meinte in seinen Bart murmelnd: genau. die haben keine Ahnung...

[HeZa]

Zitat:

Zitat von MyRealName (Beitrag 1470446)

Aber das SetText selbst ist ja eh schon ein Validator-Event und der Aufruf einer funktion in diesem Event, was eh nur eine Sache macht

OnSetText ist mitnichten ein Validator-Event. Das Validator-Event für TField lautet OnValidate. OnSetText ist das Pedant zu OnGetText. Der ursprüngliche Sinn der beiden war, ein anderes Format anzeigen bzw. bearbeiten zu können, als in der DB gespeichert ist.

Aber man kann da auch valedieren (ich glaube OnValidate kam später).

Ciao HeZa

[sakura]

Zitat:

Zitat von Lemmy (Beitrag 1470469)

das ist doch alles akademisch, das kann man doch in einem "richtigen" Projekt gar nicht nutzen... Und sein Kollege meinte in seinen Bart murmelnd: genau. die haben keine Ahnung...

Oh, was meine Kollegen da wohl sagen... :stupid:

...:cat:...

[Lemmy]

Keine Ahnung, musst sie mal fragen *fg*

Aber ich bin sicher, der eine oder andere findet das sicherlich gut ;-)

[Jumpy]

Diese Aussage ist aber absolut nachvollziehbar. Nur weil man so einen Vortrag gesehen hat oder so ein Buch gelesen hat wird man nicht zum Clean Code Jünger. Auch wenn man die Vorteile wie Testbarkeit usw. nachvollziehen kann liegt es in der Natur der Sache, dass die Konzepte immer an einfachen Beispielen erläutert werden und man bei sich denkt "aber wie soll das den bei der komplizierten Bolognese, die wir zuhause haben funktionieren".

Ist genau wie in der OOP. Da lernt man in der Schule von TPKW und TLKW abgeleitet von TFahrzeug usw. und dann soll man mal sein erstes Business-Objekt für eine konkrete Real-Life-Aufgabe erstellen und denkt sich auch, wie soll ich da jetzt ein Objekt draus schneiden.

Es wird nicht immer klar, dass man wie bei jedem Elefanten mit dem ersten Biss anfangen muss, und das man erst nach und nach versteht, wie man das vernünftig macht. Und das man es später dann nochmal anderes machen würde, weil man nun mehr weiß.
Es wird einem immer die tolle Landschaft des CleanCode in einer Dia-Show gezeigt, wenn man so will, aber eigentlich ist Clean Code ein Weg und Wege muss man gehen und das dauert Zeit.

Ich fand das Buch Clean Code für Dummies in der Beziehung ganz gut, da das dort nicht beschönigt wird und genau diese "Einstiegshürde", nenn ich sie mal, vorgestellt wird und gesagt wird: Da muss man durch. Erst wird es schlimmer bevor es gut wird.

[Lemmy]

Zitat:

Zitat von Jumpy (Beitrag 1470477)

Diese Aussage ist aber absolut nachvollziehbar.

Nö.

Zitat:

Zitat von Jumpy (Beitrag 1470477)

Nur weil man so einen Vortrag gesehen hat oder so ein Buch gelesen hat wird man nicht zum Clean Code Jünger.

Völlig richtig. Nur, wenn alle anderen seit Jahren (Jahrzehnten) davon sprechen, dass ein bestimmtes Vorgehen in definierten Situationen Vorteile besitzt, dann tut man gut daran mal inne zu halten und zu prüfen, warum man selbst immer noch vom Gegenteil überzeugt ist.

[jfheins]

Zitat:

Zitat von MyRealName (Beitrag 1470446)

"IsValidValue". Aber das SetText selbst ist ja eh schon ein Validator-Event und der Aufruf einer funktion in diesem Event, was eh nur eine Sache macht, bringt mir keinen Mehrwert an Leserlichkeit.
Ich kann es natürlich ändern wie es der Reviewer möchte, aber es geht ja um das Grundsätzliche...

Gedanken dazu ?

Ich würde als Grenze klar die Testbarkeit empfehlen. Also wenn du es so wie es ist gut testen kannst: Passt schon.
Wenn du aber dafür Verrenkungen machen musst, weil dein Form eine Instanz von TSettings benötigt, du musst das Form instanziieren damit du die Methode aufrufen kannst... :arrow: auslagern

Die Anekdote dazu: Stell dir vor du gehst zum Arzt, der sagt dir: Du brauchst einen Schrittmacher. Du fragst den Arzt: "Haben sie den denn schon getestet?" Antwort: "Nein, wir haben ein neues Design genutzt, aber das ist leider nicht testbar." :stupid:

Ergo: Je einfacher die Logik zu testen ist, desto besser ist die Architektur ;-)

Und meine Meinung dazu: Logik ohne Test = Es ist offenbar egal ob es funktioniert oder nicht.

[freimatz]

BTW: das mit den Schrittmacher ist leider ein Thema. Ich kenne jemand der die Software eines Herstellers gesehen hat. :kotz::shock: