Wenn man Units schreibt, die auch von anderen verwendet werden sollen, muß eine vernünftige Dokumentation her.
- APIs sind eine Sache für sich. Da geb ich dir recht: Die Wahrscheinlichkeit, dass dort Kommentare/Doku nötig bzw. sinnvoll ist, ist um einiges höher
- Dennoch sollten auch und gerade bei APIs die Bezeichner gut gewählt sein und die Doku im Idealfall "unnötig" machen.
- Bei APIs sollte man auch Kleinkram, Randbedingungen, etc. dokumentieren. Denn darauf kommt es an. Ein pauschales "Mach an jede Methode Doxygen/Docomatic (o.ä.) dran" ist aber kontraproduktiv. Ich hab viel zu viel Doku gelesen wie
Delphi-Quellcode:
/// Gets the value.
/// @Retrurn the value
function GetValue: Integer;
Solche Doku ist nicht nur nicht hilfreich. Sie ist dumm. Ich darf das sagen, denn ich hab sowas schon zur Genüge selbst produziert. Falsch verstandener Zwang zur Doku führt zu schlechter Doku. Die Delphi-Hilfe zur
RTL, etc. zeigt, wie man gute Doku schreibt. Aber das ist auch eine
API die von tausenden Entwicklern benutzt wird. Da darf kein Zweifel herrschen. Aber, wenn man sich selbst auferlegt für jede private Methode n Kommentar zu schreiben, macht man *definitiv* etwas falsch.
Zitat:
Der Code folgender Funktion ist simpel und relativ kurz. Trotzdem gibt es im interface Teil der
Unit Kommentare dazu.
Wie kurz oder lang die Implementierung ist, ist egal. Zumindest für die Doku [1]. Die Frage ist, welche Infos der Aufrufer braucht.
Aber wenn du schon ein Beispiel lieferst, will ich es auch nutzen. Denn es unterstreicht meine Aussage von oben.
Wenn ein Kommentar nötig ist, bedeutet das in 98,57% aller Fälle, dass entweder der Name falsch ist oder die Methode tut zu viel/das Falsche und sollte aufgeteilt werden bzw. etwas anderes tun. Sei mir nicht böse, wenn ich dein Beispiel jetzt total zerpflücke:
Delphi-Quellcode:
// Ändert Enabled von allen Controls die AControl als Parent haben (rekursiv)
Dann nenn die Methode auch so:
SetEnabledOfChildControlsRecursively
. Und schon hat sich der Kommentar erübrigt. Ja, der Name ist lang. Aber du musst ihn nur einmal tippen. Die Code-Vervollständigung sollte ja den Rest übernehmen. Dafür sparst du dir das Nachgucken in der Doku und dein Code wird deutlich besser lesbar.
Delphi-Quellcode:
// Parameter: AForm: Formular auf dem der Container AControl liegt
- Wenn das ein Formular sein muss, sollte auch der Typ TCustomForm sein.
- Und viel wichtiger: Der Parameter sollte gar nicht nötig sein (
IAP).
Delphi-Quellcode:
// AControl: Container, der andere Controls beinhaltet
- Ich bin nicht mehr ganz so firm in der
VCL, aber ist es nicht TWinControl, das ein Container sein kann? Korrigiere mich, wenn ich falsch liege. In dem Fall sollte der Typ des Parameters TWinControl sein.
- Angenommen, da stände TWinControl. Damit wäre der Kommentar redundant zur Doku von TWinControl. Also ist der in der Form überflüssig.
Delphi-Quellcode:
// AValue: True=alle werden Enabled <p>False=alle werden Disabled
Wenn der Name der Methode wie oben vorgeschlagen ist, ist der Kommentar hier auch überflüssig, denn er wiederholt die Funktionsweise der Methode. Du wolltest AValue erklären. Die Korrekte Beschreibung wäre "Der Wert auf den die Eigenschaft Enabled von allen ChildControls gesetzt werden soll". Und das sagt bereits der Name.
Delphi-Quellcode:
procedure SetEnabledRec(AForm: TComponent; AControl: TControl; AValue: Boolean);
"Rec" ist zwar kurz, aber mehrdeutig. Record? Recording? Recorded? Recently? Anders als
Dec und Inc ist Rec nicht unbedingt eine allgemein bekannte Abkürzung. Und Abkürzungen in Bezeichnern, die nicht eh klar sind, sind schlecht.
Jetzt mal angenommen, das sollte Teil einer
API für viele Entwickler sein. So, wie die
RTL eben. In dem Fall fehlen mir wichtige Infos. Du hast die Methode nicht vollständig beschrieben:
- Wird Enabled von AControl auch gesetzt oder nur von den Kindern?
- Wird die Parent- oder die Owner-Hierarchie der Komponenten als "ist enthalten in" betrachtet. Ich gehe mal fest davon aus, du meinst letztere. Wenn du aber
API schreibst, solltest du da genau sein.
- Was passiert, wenn AForm und AControl nicht zusammen passen? Wie oben geschrieben solltest du das eh verhindern. Wenn du es nicht verhindern kannst -- warum auch immer --, musst du es dokumentieren.
Man kann jetzt noch weiter überlegen. Will man tatsächlich einen Boolean übergeben? Oder will man nur alle entweder an oder aus knipsen. Wenn ersteres hinreichend selten ist, kann man auch überlegen, die Methode zu teilen:
EnableChildControlsRecursively(TWinColntrol)
und
DisableChildControlsRecursively(TWinControl)
. Das würde das noch einfacher machen, das zu benutzen. Genau genommen könnte man die Methoden im Zweifel einfach zusätzlich anbieten. Damit stellt sich die Frage gar nicht mehr. Also auf jeden Fall teilen.
Als nächstes bemerke ich, dass dadurch, dass das Recursively ausgeschrieben ist und nur noch ein Parameter da ist, das "ChildControls" fast schon wieder überflüssig wird. Also entferne ich das wieder (Refactoring ist eben ein iterativer Prozess). Damit wäre ich bei
EnableRecursively(TWinColntrol)
und
DisableRecursively(TWinControl)
. Ich glaub, jetzt bin ich endlich zufrieden.
Und jetzt vergleichen wir mal:
Delphi-Quellcode:
SetEnabledRec(Self, panel1, false);
// vs.
DisableRecursively(panel1);
mfg
Christian
P.S.: Hier ein sehr schönes Video, das zeigt, wie man u.a. durch Entfernung von unnötigen Kommentaren Code besser macht:
http://www.youtube.com/watch?v=aWiwDdx_rdo
[1] Davon abgesehen sollte quasi jede Methode kurz sein. Aber das steht hier nicht zur Debatte.