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