Docs: StyleGuide.md as single source of truth for coding style

[omonien]

Summary

• Adds Docs/StyleGuide.md as the single source of truth for DEC coding style, structure, and contribution expectations for new / rewritten code.
• Consolidates former scattered notes from CONTRIBUTING.md, DEC65.pdf §3.7.1 (structure & style), de-facto library conventions (DEC… units, Apache headers, FPC/Delphi uses), and adapted practices from the modern Delphi Style Guide (naming prefixes, formatting, XML docs).
• Reworks CONTRIBUTING.md into a short process entry point that points at the style guide.
• Updates readme.md so contributors find the style guide next to the other project docs.

Policy (explicit)

• No production / existing source reformatting in this PR (docs only).
• New code and substantial rewrites (including future feature PRs such as ChaCha re-land) should be reviewed against Docs/StyleGuide.md.
• Algorithm how-to (DEC65.pdf §3.7.2+) stays in the PDF; when the PDF is next regenerated, §3.7.1 can shrink to a pointer at Docs/StyleGuide.md.

Test plan

• Docs-only review: read Docs/StyleGuide.md for DEC fit (FPC, unit naming, headers).
• Confirm CONTRIBUTING.md and readme.md links resolve.
• No .pas / build changes — nothing to compile.

[MHumm]

I'm currently reading the styleguide, as far as I am it's mostly good. Eight things:

1. What does re-lands in "donor re-lands such" mean?

2. This example
   '''procedure TExample.DoWork(const AValue: string);
   begin
   if AValue = '' then
   raise EDECException.Create('Value required');

if IsReady then
begin
Process(AValue);
end;
end;'''

Is not too clever. In this example I wouldn't wrap Process(AValue); in begin / end, as this is simply unnecessary.
We should put at least another command in this begin / end block and describe that such a single call shouldn't get wrapped as that's simply superflous.

3. I would ammend this one like shown here: Non-portable code | Wrap in {$IFDEF} so FPC (or unsupported platforms) do not see it. Use DECOptions.inc as the central place for turning on/off specific defines you added.
   So it's clear where such defines like classregistration etc. should be turned on/off/"defined".

4. I wouldn't write "avoid withs". I would simply forbid it ;-) We're not C-programmers trying to save each keypress ;-)

5. The list of Type and Interface prefixes misses simple type, e.g. if somebody declares TIndex = Integer the T prefix should be used as well, not only for records.

6. If we mention this: Record public fields have no F prefix (they are part of the value layout). we would mention this for other public fields as well. Where's the value of this sentence? No Public field should have a F prefix. If the word itsself starts with F this is kept of course. Example: property FileName: string...

7. Constants: Where was this one deduced from? String constant | sc | scInvalidNonce
   There are some string constants starting with sC, but only because the 1st word of the constant is Cipher.
   sCipherSomething... So I'd change this to string constants start with s.

8. The example in Documentation comments does not describe AInitVector. For some it might be an obvious parameter for others it might not be (e.g. folks new to cryptography) so it would be better to have a rule to document all parameters.

[omonien]

Hallo Markus,

danke fürs sorgfältige Lesen. Kurz zu allen acht Punkten — alles StyleGuide für neuen Code, kein Massen-Rewrite der bestehenden Units.

1. „re-lands“ war unglückliches Englisch. Gemeint: Code, der über einen Pull Request kommt („donor“), muss dem StyleGuide folgen. Beispiel ChaCha20/Poly1305 aus Basically a first implementation of the chacha20 Poly1305 algorithm #90: Wenn wir das später übernehmen, räumen wir Naming/Struktur/Docs nach dem Guide auf, statt den Donor-Stil 1:1 zu kopieren. Formuliere ich im Guide schlichter.

2. Das begin/end um eine einzelne Anweisung ist Absicht. Bei Code, den mehrere Leute pflegen, zählt Eindeutigkeit mehr als Kürze — auch wenn es länger aussieht. Klassiker: zweite Zeile eingefügt, Einrückung wirkt richtig, gebunden ist nur die erste. Ausnahme bleiben nacktes Exit/raise/Continue/Break. Beispiel und Satz im Guide schärfe ich so, dass die Absicht klar wird.

3. Einverstanden: eigene Defines zentral in DECOptions.inc — nehme ich in die Non-portable-Zeile auf.

4. Einverstanden: with wird verboten, nicht nur „avoid“.

5. T bei Typaliasen machen wir zur Empfehlung, kein Muss. Oft sinnvoll (TIndex = Integer), manchmal aber unpraktisch (RTL-artige Aliase, z. B. String-Namen ohne T).

6. Hier gehe ich etwas weiter als die reine Namensfrage: Öffentliche Klassenfelder sollten vermieden werden (Properties statt Felder). Kein hartes Verbot wegen der historischen Codebasis, und keine Namensregel für öffentliche Felder — sonst legalisieren wir sie durch die Hintertür. Gründe: sie wirken wie Globals (State ohne klare Get/Set-Zuständigkeit), Feld vs. Property ist im Code unklar, keine Schreib-/Leserechte. Viele öffentliche Felder deuten oft eher auf ein Record (oder Architekturbruch) hin. Bei Records bleiben öffentliche Felder normal (Wertlayout).

7. Einverstanden: String-Konstanten einheitlich mit s…, wie in DEC üblich. sc war missverständlich (sCipher… ist „s“ + Cipher, kein Prefix sc).

8. Einverstanden: Bei neuer public API alle Parameter dokumentieren. Beispiel im Guide ergänze ich um AInitVector.

Wenn das für dich passt, arbeite ich das im StyleGuide nach.

Viele Grüße
Olaf

[MHumm]

Ja bitte nacharbeiten. Geht in die richtige Richtung.
Mir kam noch ein Ergänzungsvorschlag:
Den Einbau von Asserts für alle Objektreferenz Parameter fordern die nicht als optional definiert sind.
Bei normalen Parametern würde ich das nicht hart fordern, da manchmal nicht ganz klar ist was erlaubt und was außer dem Wertebereich ist, v.a. bei Routinen die allgemeinerer Natur sind.

Vielleicht so was in Kapitel 5.2 Variables and parameters:

Asserts
Non optional object reference or pointer typed parameters must be guarded with Assert(Assigned(ParamName), 'ParamName is not assigned');

[omonien]

Hallo Markus,

die acht Punkte sind im StyleGuide schon nachgezogen (8f9049f auf diesem PR).

Zu den Asserts: Die Richtung „nicht-optionale Objekt-/Pointer-Parameter prüfen“ teile ich. Von Assert(Assigned(...)) würde ich aber dringend abraten zugunsten einer harten Prüfung mit EArgumentNilException (oder der DEC-üblichen EDEC*-Variante, falls wir die schon so nutzen).

Grund: Assert fällt im Release typischerweise weg ({$C-} / Assertions aus). Die if not Assigned then raise-Variante kostet zwar auch im Release eine Abfrage, aber in einer Crypto-Bibliothek zählen Robustheit und das Vermeiden von Nil-Pfaden/Exploits mehr als diese Mikro-Optimierung. Bei normalen Wertparametern bleibe ich bei deiner Linie: nicht hart fordern.

Wenn das für dich passt, nehme ich die Regel so in §5.2 auf.

Viele Grüße
Olaf

[MHumm]

Ok! Olaf Monien ***@***.***> schrieb am Mi., 9. Sept. 2026, 14:10:

…

*omonien* left a comment (MHumm/DelphiEncryptionCompendium#101) <#101 (comment)> Hallo Markus, die acht Punkte sind im StyleGuide schon nachgezogen (8f9049f auf diesem PR). Zu den Asserts: Die Richtung „nicht-optionale Objekt-/Pointer-Parameter prüfen“ teile ich. Von Assert(Assigned(...)) würde ich aber dringend abraten zugunsten einer *harten* Prüfung mit EArgumentNilException (oder der DEC-üblichen EDEC*-Variante, falls wir die schon so nutzen). Grund: Assert fällt im Release typischerweise weg ({$C-} / Assertions aus). Die if not Assigned then raise-Variante kostet zwar auch im Release eine Abfrage, aber in einer Crypto-Bibliothek zählen Robustheit und das Vermeiden von Nil-Pfaden/Exploits mehr als diese Mikro-Optimierung. Bei normalen Wertparametern bleibe ich bei deiner Linie: nicht hart fordern. Wenn das für dich passt, nehme ich die Regel so in §5.2 auf. Viele Grüße Olaf — Reply to this email directly, view it on GitHub <#101?email_source=notifications&email_token=AHCTXHKJSGRX2HL4WQBIFG35OFCEBA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNRQGE3DAMBZGMYKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5601600930>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHCTXHPUXU6YNFQYECZZOCT5OFCEBAVCNFSNUABEKJSXA33TNF2G64TZHMZTENJVGIZDMNJ3JFZXG5LFHM2DSNJYGQ4DENZZGSQXMAQ> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AHCTXHKGXDGH6EPARG3EPRL5OFCEBA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNRQGE3DAMBZGMYKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android <https://github.com/notifications/mobile/android/AHCTXHOAUM4UUZGPRD7FK535OFCEBA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNRQGE3DAMBZGMYKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today! You are receiving this because you commented.Message ID: ***@***.***>

[omonien]

Hallo Markus,

super, danke fürs OK.

Dann ist alles wie besprochen auf diesem PR nachgezogen:

• die acht StyleGuide-Punkte (8f9049f)
• §5.2 Parameter guards: harte Nil-Prüfung mit EArgumentNilException / Assigned statt Assert (33aed0d) — für nicht-optionale Objekt-/Pointer-/Interface-Parameter; Value-Parameter und TBytes bewusst ausgenommen; nur neuer Code

Damit ist #101 aus meiner Sicht fertig zum Mergen.

Viele Grüße
Olaf