Was gehört alles in eine richtige Dokumentation?

[r2c2]

Zitat von Graberius:

Sichwort Dokumentation
Egal ob du jetzt Object Pascal, C/C++, Java, Groovy, Velocity, HTML oder JavaScript Programmierst...
Kommentier wenigstens was eine Methode oder Funktion machen soll!!!

Besser: Schreib deinen Code so, dass klar ist was er tut. Was eine Methode tun soll, sollte ihr Name verraten. 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.

[MaBuSE]

Zitat von r2c2:

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.

Das sehe ich anders.

Wenn man Units schreibt, die auch von anderen verwendet werden sollen, muß eine vernünftige Dokumentation her.

Beisp.:

Der Code folgender Funktion ist simpel und relativ kurz. Trotzdem gibt es im interface Teil der Unit Kommentare dazu.

Delphi-Quellcode:

// Ändert Enabled von allen Controls die AControl als Parent haben (rekursiv)
// Parameter: AForm: Formular auf dem der Container AControl liegt
//            AControl: Container, der andere Controls beinhaltet
//            AValue: True=alle werden Enabled <p>False=alle werden Disabled
procedure SetEnabledRec(AForm: TComponent; AControl: TControl; AValue: Boolean);

Im implementation teil stehen dann normalerweise keine Kommentare mehr. Es sei denn es wird etwas gemacht, was auf den ersten Blick nicht ersichtlich ist (Trick, Workarround, ...) das wird natürlich direkt im Code kommentiert.

Docomatic macht übrigends auf dem Kommentar oben bei und folgenden Text in der Systemdokumentation:

[r2c2]

Zitat von MaBuSE:

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.

[MaBuSE]

Zitat von r2c2:

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:

Nein, böse bin ich nicht, aber das war nur ein Beispiel, dass ich auf die Schnelle rausgesucht habe.
Ich hatte eine Funktion gesucht, die nicht zu viel Kommentar / Dokumentation hat.
Diese Funktion ist Teil einer Bibliothek, die schon seit mehr als 15 Jahren in Benutzung ist. Sie wird von ca. 40 Entwicklern in unzähligen Projekten verwendet.

Es traut sich einfach niemand so alte und häufig benutze Funktionen "anzufassen".

Aus diesem Grund wurde sie vor ein paar Jahren wenigstens dokumentiert

Ich glaube es gibt sogar eine Funktion DisableRecursive(Form.Control); , welche die alte aufruft

In deinen Ausführungen gebe ich dir größtenteils (ich bin kein Fan von sehr langen Namen) Recht.
Der Entwickler der Funktion würde das heute wahrscheinlich auch anders machen.

Trotzdem würde ich auch bei einer eindeutig benamten Funktion, mit eindeutig benamten Parametern, eine Dokumentation erwarten.
Auch wenn in der Dokumentation "nur" in Prosa das steht, was man beim Namen der Funktion erwarten würde.

Ich bin auch Fan von Änderungshistorien.
z.B: In der Funktion hat sich ab 01.01.2013 folgendes Verhalten geändert: ...
Oder auch der Autor und das Erstellungsdatum einer Funktion.
Diese Informationen bekommt man zwar auch z.B. über ein Blame in git, aber solche Dinge gehören für mich einfach in eine Dokumentation. (Zumindest bei APIs)

[Furtbichler]

Zitat von r2c2:

Wenn ein Kommentar nötig ist, bedeutet das in 98,57% aller Fälle, dass entweder der Name falsch ist ...

Zitat von MaBuSe:

Das sehe ich anders.

Delphi-Quellcode:

// Ändert Enabled von allen Controls die AControl als Parent haben (rekursiv)
// Parameter: AForm: Formular auf dem der Container AControl liegt
//            AControl: Container, der andere Controls beinhaltet
//            AValue: True=alle werden Enabled <p>False=alle werden Disabled
procedure SetEnabledRec(AForm: TComponent; AControl: TControl; AValue: Boolean);

Also, ich würde mal sagen, der Name 'SetEnabledRec' ist Schrott. Im Clean-Code sollte er so heißen (z.B.):
SetEnabledOfChildControlsOnForm(aForm : TComponent; aParentControl : TControl; aValue : Boolean); Wozu also jetzt noch Kommentare?

[DeddyH]

Nicht
SetChildControlsOnFormEnabledRecursively(aForm: TCustomForm; aParentControl: TWinControl; Enabled: Boolean);

[MaBuSE]

Zitat von Furtbichler:

Also, ich würde mal sagen, der Name 'SetEnabledRec' ist Schrott. Im Clean-Code sollte er so heißen (z.B.):
SetEnabledOfChildControlsOnForm(aForm : TComponent; aParentControl : TControl; aValue : Boolean); Wozu also jetzt noch Kommentare?

Ich möchte dazu nur folgendes sagen:

1. Hat r2c2 das in seinem sehr ausführlichen Beitrag schon wirklich nett erklärt.
2. In Deinem Namen nicht erkennbar ist, dass rekursiv die Eigenschaft Enabled aller untergeordneten Elemte gesetzt werden.
3. Habe ich geschrieben, warum das so ist, und ich auch nicht glücklich damit bin.
4. Werden die Kommentare zur Erstellung der Dokumentation verwendet.
   Und ich bin immer noch der Meinung, dass in der Beschreibung einer API auch eindeutige Funktionen dokumentiert werden sollten.
   z.B. die Funktion abs()-> http://docwiki.embarcadero.com/Libra.../de/System.Abs

[Furtbichler]

...Hier stand der Beginn einer kleinen Auseinandersetzung, der jedoch von MaBuSe per PN im Keim erstickt wurde, weswegen das auch nicht hierhin gehört...

Ich fand es nur lustig, das r2c2's (korrekte) Aussage, sprechende Namen seien besser als Kommentare neben dein Paradebeispiel schlechter Nomenklatur zu stellen. Ich meine, das muss Dir doch selbst aufgefallen sein, das der Name ähem.. du weisst schon. Auto-an-Wand.

Übrigens, falls es dich interessiert: Ein Methodenname darf gerade nicht den Algorithmus ('Rekursiv') wiederspiegeln, sondern kurz und knapp erklären, was die Methode anstellt. 'SetEnabledRec' ist Schrottungenau, 'SetEnabledStateOfChildControls' nicht. Ob das rekursiv, iterativ oder sonst was ist, ist irrelevant. Wir können natürlich auch richtig albern werden, und das Teil
'SetEnabledStateOfChildAndTheirChildsAndTheirChild sAndSoOnControls' nennen, aber -hey- WTF.

Eine API zu dokumentieren, ist notwendig und vollkommen unabhängig von meinem Einwand (ich schrieb eigentlich nichts davon).

WEnn Du deinen API-Funktionen, oder wer auch immer die so genannt hat, einfach aussagekräftige Namen gibst, wird eine Dokumentation schnell überflüssig. Das merkst Du daran, das Du für die Beschreibung der Methode fast genau die Worte verwendest, die im Namen der Methode selbst stecken:

Delphi-Quellcode:

// Creates a Form of type aFormClass
procedure UIManager.CreateForm(aFormClass :TFormClass)

Es gibt Kollegen, die sich weigern das so hinzuschreiben. Weil es so einfach nur kreuzdämlich ist. Und eine Methode nur um des Kommentieren willens zu kommentieren, erinnert mich an die Schildbürger.

Dessenungeachtet sind Konstrukte wie dieses 'Ich setze Enabled gleich für alle Childcontrols' eigentlich kein Fall für eine API, denn eine API soll ja nur die Basisfunktionen bereitstellen und keine Superdupersonderichmachdasallesfürdich-Funktionen. Aber das steht auf einem anderen Blatt.

--- und hier stand auch noch überflüssiges Zeugs---

[r2c2]

Zitat von MaBuSE:

Es traut sich einfach niemand so alte und häufig benutze Funktionen "anzufassen".

Das macht APIs so schwer: Wenn man sie rausgibt, bedeutet das gleichzeitig, dass man sie partiell in Zement giest. Eine API nachträglich zu ändern ist nicht mehr so einfach möglich. Aber es gibt Möglichkeiten: Schreib ne schöne, neue Methode und mach die alte deprecated. Und in 10 Jahren kannst du vielleicht drangehen, die alte zu entfernen. Das ist langwierig, ja. Aber, wenn du das jetzt nicht tust, kannst du dir sicher sein, dass du die Methode selbst in 10 Jahren auch nicht los sein wirst.

Zitat:

Aus diesem Grund wurde sie vor ein paar Jahren wenigstens dokumentiert

Schonmal besser als nichts, ja. Aber das ist natürlich keine Heilung. Nur ein Pflaster.

Zitat:

Ich glaube es gibt sogar eine Funktion DisableRecursive(Form.Control); , welche die alte aufruft

Guggemalda. Dann fehlt jetzt nur noch das deprecated.

Zitat:

Trotzdem würde ich auch bei einer eindeutig benamten Funktion, mit eindeutig benamten Parametern, eine Dokumentation erwarten.
Auch wenn in der Dokumentation "nur" in Prosa das steht, was man beim Namen der Funktion erwarten würde.

Einfache Frage: Warum?

Zitat:

Ich bin auch Fan von Änderungshistorien.
z.B: In der Funktion hat sich ab 01.01.2013 folgendes Verhalten geändert: ...
Oder auch der Autor und das Erstellungsdatum einer Funktion.
Diese Informationen bekommt man zwar auch z.B. über ein Blame in git, aber solche Dinge gehören für mich einfach in eine Dokumentation. (Zumindest bei APIs)

- Kommentare sind keine schlechte Versionsverwaltung. Kommentare sind *etwas anderes*. Da die selben Infos rein zu packen, macht keinen Sinn.
- "gehört für mich einfach rein" ist kein Grund. Wenn ich bemerke, dass ich für eine Meinung keine Gründe habe, stelle ich diese in Frage. Ich kann das nur empfehlen, denn es ist augenöffnend.
- Was bei APIs sinnvoll sein kann (aber nicht muss): Dokumentieren, ab welcher Version etwas etwas neu eingeführt worden ist. So kann man beispielsweise nachgucken, ob Code noch mit ner alten Bibliothek laufen würde. Sowas wie "Läuft meine Komponente auch noch unter Delphi 7?"
- Ähnlich: Breaking changes: Läuft mein Code mit der neuen API-Version noch?

Warum sage ich eigentlich immer "sinnvoll sein kann"? Ganz einfach: Wie in meinem ersten Beitrag hier im Thread geschrieben, kommt es immer auf die Stakeholder drauf an. Doku machen wir aus einem Grund. Nämlich, dass sie gelesen wird. Doku macht Arbeit. Arbeit zu schreiben, Arbeit zu pflegen und ja, auch Arbeit zu lesen. Wenn ich jetzt Doku habe, die keinen Mehrwert bietet, sondern genauso schlau bin, nachdem ich sie gelsenen habe, wie zuvor, hätte ich mir gar nicht erst die Mühe machen müssen, sie zu lesen. Es ist frustrierend, Doku zu lesen, die keinen Mehrwert bietet und nich frustrierender, wenn man sich vor Augen hält, dass jemand Zeit darin investiert hat, mir dadurch Zeit zu rauben...

Zitat:

Und ich bin immer noch der Meinung, dass in der Beschreibung einer API auch eindeutige Funktionen dokumentiert werden sollten.
z.B. die Funktion abs()-> http://docwiki.embarcadero.com/Libra.../de/System.Abs

Was ein Beispiel für eher weniger gute Doku ist. Was man von einer *wirklich* guten Doku erwarten würde:
- Dass sie auch Leute verstehen, die nicht wissen, dass "Absolutwert" ein Synonym für die Betragsfunktion ist
- Dass sie keine Infos wiederholen, die in der Signatur stehen ("X ist ein Ausdruck des Typs Integer oder Real.")
- Dass Randfälle erklärt werden. Was passiert, wenn man NaN übergibt? Exception oder auch NaN? Das wären Infos, die einen Mehrwert bieten.

Zitat von Furtbichler:

Übrigens, falls es dich interessiert: Ein Methodenname darf gerade nicht den Algorithmus ('Rekursiv') wiederspiegeln, sondern kurz und knapp erklären, was die Methode anstellt.

Hier ist "Recursively" Teil der Funktionsbeschreibung. Es werden auch Kindeskinder betrachtet. Das ist keine Aussage über den Algo, der dahinter steckt. Auch der kann rekursiv sein (und es es hier vermutlich auch). Aber er muss es nicht. Fakt ist, dass "rekursiv" nicht nur einen Algo beschreibt, sondern auch eine funktionale Anforderung sein kann.

[Furtbichler]

Zitat von r2c2:

Zitat:

Trotzdem würde ich auch bei einer eindeutig benamten Funktion, mit eindeutig benamten Parametern, eine Dokumentation erwarten.
Auch wenn in der Dokumentation "nur" in Prosa das steht, was man beim Namen der Funktion erwarten würde.

Einfache Frage: Warum?

Weil erstens eine System/API-Doku vollständig sein sollte und zweitens eine Doku erweitert werden sollte. Keine Doku wird selten erweitert. Eine API, die aus elementaren Operationen besteht, die aussagekräftige Namen haben ist selbsterklärend. Das ist die Krux bei guten API: Einerseits gehört die Doku dazu, weil dort eben auch steht, was für Seiteneffekte es gibt, Beispiele, Exceptions, der ganze Kram eben, Und da sieht es eben blöd aus, wenn in der Doku steht: "CreateForm: Guess what the function does..." oder "The name says it all" (obwohl, warum eigentlich nicht).

Ach, und die Sache mit der Rekusivität im Methodennamen kann man so sehen, muss man nicht, aber dein Einwand ist schon ok.