An welcher Stelle die Parameter validieren

[starcow]

Ich brauche mal wieder eure Meinung / Erfahrung zu folgender Situation:

Ich habe eine Translation Unit, die nach aussen hin zwei Funktionen anbietet:

Code: Alles auswählen

void func_a( ... );
void func_b( ... );

Diese Funktionen nutzen intern die selbe static funktion

Code: Alles auswählen

static foid func_static( ... );

Wo würdet ihr jetzt die Parameter validieren, die die Funktionen func_a und func_b erhalten, wenn diese Parameter an die static Funktion weitergereicht werden?

Szenario 1:
die Parameter werden in func_a und func_b validiert, da diese die öffentliche API definieren.
Das ist quasi die Vertrauensgrenze. Ungültige Werte gelangen erst gar nicht weiter ins System hinein.
static functions können somit immer auf sichere Werte vertrauen.

Szenario 2:
die Parameter werden erst in func_static validiert. Dadurch finden die checks nur einmal, an einer zentralen Stelle statt.
Würde später eine dritte Funktion func_c dazukommen, könnten dort die Checks nicht vergessen werden.

Szenario 3:
Die Checks finden an beiden Stellen statt.

Persönlich finde ich Szenario 3 eher irritierend.
Szenario 1 und 2 haben aber irgendwie beide etwas.
Die "Vertrauensgrenze" fühlt sich irgendwie natürlich an. Falsche Werte werden damit so früh wie möglich abgewiesen oder korrigiert.
Allerdings ist der Aspekt mit der "Code Duplikation" und dem späteren Vergessen eines checks in Szenario 2 auch ein sehr starkes Argument.

[Jonathan]

Ich bin ja immer für robusten Code, weil man sich sonst immer verfranzt und am Ende was kaputt geht. Wenn func_a die Parameter nicht benutzt sondern nur weiter reicht, kann da ja egal sein, welcher Wert drin steht. Wenn func_static wirklich auf korrekte Werte angewiesen ist, sollte da auch getestet werden. Wenn du immer testen willst, sollte es eine zentrale Stelle geben, an der getestet wird, sonst erhöht sich die Chance, dass der Test verpasst wird. Außerdem, vlt. musst du den Test ja mal anpassen - erinnerst du dich dann an alle Duplikate?

Szenario 3 wäre ok, wenn du einen early-out wählen kannst: Ich teste die Parameter, weil ich mir ggf. die Vorberechnungen komplett sparen kann, bevor ich func_a aufrufe. Allerdings ist dann auch die Frage, welche Art von Fehlern du meinst. Wenn ich Exceptions schmeiße, ist mir sehr oft egal, ob das Programm danach tatsächlich weiterlaufen kann, weil die zu 95% bedeuten, dass ich mich verprogrammiert habe. D.h. ich muss den Fehler beheben und das Programm neu kompilieren oder zumindest neu starten (wenn in einer config ein Wert fehlt, muss die angepasst werden, es gibt keine sinnvolle Möglichkeit, das Programm vorher weiter auszuführen).

Danach kannst du halt auch unterscheiden. Laufzeitabfragen, die im normalen Betrieb vorkommen sollten ("kann Objekt nicht löschen, da keines vom User ausgewählt wurde") würde ich tendenziell so früh wie möglich abfragen, Bugs dagegen so zentral wie möglich (also vermutlich sehr spät). In dem Beispiel gäbe es also 2 Abfragen: Eine im Userinterface, die eine Box mit Fehlermeldung öffnet, und eine intern (die keine Box öffnen kann, weil intern), die eine Exception schmeißt und das Programm beendet, weil die Schnittstelle zur internen Komponente falsch verwendet wurde. Das ist ein Programmierfehler und erfordert neukompilieren (in dem Falle das hinzufügen der GUI-Meldung).

[Krishty]

An dieser Frage kauen Software-Designer seit Jahrzehnten.

Persönlich mache ich Szenario 3, auch wenn’s schmerzt. Validierung an API-Grenze ist ein Muss. Die interne Funktion ist ein Implementierungsdetail und sollte deshalb nicht die API-Eingaben validieren, sondern nur ihren eigenen Parametersatz, der ja mehr oder weniger zufällig mit der API übereinstimmt.

Der historische Hintergrund ist beschrieben in The Rise of Worse is Better. Es gab damals (in den 80ern) zwei Schulen: MIT und Unix. MIT besteht darauf, dass Schnittstellen sich korrekt zu verhalten und einfach zu benutzen sein müssen, und dass Implementierungsdetails nicht nach außen dringen – Szenario 1 bei dir.

Die Unix-Schule legt stattdessen wert darauf, dass die Implementierung einfach ist, selbst wenn die Schnittstelle dadurch nicht mehr voll abstrahiert – Szenario 2 bei dir. (Alle Aufrufer validieren jetzt gegen deine interne Implementierung in func_static() statt gegen die öffentliche Schnittstelle.)

Microsoft folgt eher der MIT-Schule, und POSIX eher der Unix-Schule. Deshalb musst du bei Linux auf so sonderbare Fehlerzustände wie EGAIN gefasst sein; bei Win32 eher nicht.

Die Vor- und Nachteile beider Lösungen sind ja im Artikel schön erläutert. Die Vorhersage dort, dass 1995 alles auf Unix und C laufen würde, hat sich erst verspätet erfüllt – als in den 2010ern Smartphones und IoT auf den Markt drängten. Heute läuft auf den meisten Chips ein Unix-Derivat, aus exakt den Gründen im Artikel.

[starcow]

Danke, das war wirklich sehr interessant zu lesen!
Das Thema (wie reagieren auf ungültige Argumente) scheint mir in der Programmierung eines der schwierigsten zu sein - gerade, weil pauschale Aussagen selten möglich sind und Entscheidungs-Dilemmen schnell entstehen können.

Wenn ich versuche, grob zusammen zufassen, wie auf falsche Argumente reagiert werden kann, dann lande ich bei folgenden Szenarien:

1) nicht prüfen, resp. nichts unternehmen. Dann ist es bei falschen Werten einfach UB.
2) das Programm terminieren oder ordnungsgemäss beenden
3) Den Wert sättigen oder einen default value zuweisen und die Funktion damit fortsetzen.
4) Auf den ungültigen Wert mittels no-op reagieren und frühzeitig returnen - ohne einen Fehler zu rapportieren.
5) Frühzeitig returnen und einen Fehler zurückgeben. Und damit die Entscheidung, was jetzt passieren soll, zurückdelegieren

Habe ich etwas übersehen?

Szenario 1) finde ich grundsätzlich bei internen static Funktionen legitim. Die Werte müssen einfach stimmen, sonst ist es UB. Stimmen die Werte nicht, hat das Programm einen Bug.
Bei Szenario 5) frage ich mich oft, was das bringen soll, wenn es eine Art von Fehler ist, die eigentlich _nie_ hätte auftreten sollen (also keine user, file, oder network Dinge). Wenn diese Invariante auftritt, dann hat das Programm einen Bug. Weiterlaufenlassen ist dann eigentlich sowieso nicht mehr sinnvoll.
Aber wo zieht man die Grenze?
Soll eine Funktion an einer API Grenze "grosszügiger" sein? Z.B. negative Werte auf 0 setzen, anstatt gleich zu terminieren?
Manches lässt sich intuitiv gut beantworten, aber manche Frage sind echt ein Dilemma.
Haben sich irgendwie sinnvolle Regeln bei euch etabliert?

[Jonathan]

Ich meine, ich bin derjenige der meine Spiele mit Abstand am häufigsten startet. Meistens während der Entwicklung. Ich will nach Möglichkeit überhaupt keine Bugs haben, also sollten standardmäßig alle Fehler laut sein (-> Exception und Programm Abbruch). Ich passe den Code an und kompilliere neu, danach geht es weiter.

Das ist für mich gut, aber für andere schlecht. Ich hab im Code den Asset-Namen falsch geschrieben, jetzt lädt im Hauptmenü ein Hintergrund-Sprite nicht. Der Benutzer kann das später nicht beheben und dann weiter machen. Für den ist hier Game Over. Für den Benutzer sollte das Programm so robust wie möglich sein, mir doch egal wenn das Hauptmenü kaputt ist, solange ich mich irgendwie ins Spiel klicken und da loslegen kann ist das besser als wenn gar nichts geht.

Werte anpassen? Selbes Prinzip. Eigentlich will ich in der GUI verhindern, dass der Nutzer Quatschwerte definieren kann. Eine Smoothstep-Funktion die zwischen 0 und 1 interpolieren soll kann ihr Argument gerne entsprechend clampen. Bei fast allem anderen will ich aber Fehlermeldungen haben. Wenn mein Charakter initial -50 HP hat, soll das nicht auf den Mindestwert von 10 gesetzt werden. Das ist offensichtlich ein Fehler und ich muss den fixen. Ansonsten ändere ich später mal die Mindest-HP und plötzlich hat mein Charakter 5 oder 20 HP, obwohl die doch eigentlich konstant bleiben sollten und meine ganze Balance ist kaputt, weil ich das nicht nochmal geprüft habe. Robust ist es, wenn alle Fehler so schnell wie möglich beseitigt werden, damit sich da gar nichts auftürmen kann.

Wieder gilt: Der Endnutzer möchte vielleicht trotzdem die Map starten können, auch wenn eines der 50 Monster vielleicht nicht ganz korrekte Werte hat.

Ich würde deshalb aber jetzt nicht unbedingt für jede Fehlermöglichkeit 2 verschiedene Wege implementieren, sondern eher beim Entwickeln entscheiden, ob ich den Fehler beim Endkunden überhaupt erwarten würde, und danach entscheiden.

[Alexander Kornrumpf]

Ich würde behaupten viel Erfahrung mit dieser Frage zu haben, insbesondere habe ich sehr viel "Szenarion 3" Code refactored, aber ich bekomme die Erfahrung nicht in eine kurze Regel gepresst (wie das so ist mit jeder Erfahrung, die irgendwas Wert ist). Ich habe versucht Links rauszusuchen für Dinge die ich einfach "Seit Jahren mache", nicht für alle Links lege ich meine Hand ins Feuer.

Mindestens zu bedenken:

Oft ist func_static einfach ein Fall von The Wrong Abstraction.

Insbesondere (Spezialfall des o. g.) ist der Tradeoff bei "Szenario 3" ja offensichtlich den Check zu duplizieren damit man die Workload nicht duplizieren sondern in func_static wegabstrahieren kann. Das ist nicht immer der richtige Tradeoff, insbesondere wenn das Verhältnis von Checks zu Workload groß wird.

Wenn zwischen Check und Workload Zeit liegt, weil etwa func_a erst die Parameter checkt, dann etwas tut und dann erst func_static aufruft und der Check gegen globalen State geht (was er realistisch regelmäßig tut, who are we kidding) hast du eine race condition in Form der Wette dass sich der globale State nicht geändert hat, seit du den Check gemacht hast. Was hier die korrekte Lösung ist kommt drauf an welche Garantien du geben willst, aber die korrekte Lösung dieses Problems wird in der Regel einen Ablauf vorgeben der orthogonal zu deiner ursprünglichen Frage steht (d.h. deine ursprüngliche "API-Grenze" war möglicherweise nicht an der richtigen Stelle, wenn du fragen musst).

Das obige zusammen erschlägt schon irgendwie 80% der Fälle. Für die anderen 20% kannst du die Duplizierung der Checks vermeiden in dem func_static ein immutables Objekt akzeptiert das bei Initialisierung gecheckt wird (und dann da immutable, immer gecheckt bleibt). In anderen Worten du musst garantieren, dass jede Initialisierung eines solchen Objekts den Check macht und dass das Objekt nach Initialisierung unveränderlich ist. C ist nicht die komfortabelste Sprache um das zu machen. Der Sinn der Übung ist dass der Aufrufer somit gezwungen wird den Check zu machen, denn er braucht ja das Objekt um den Aufruf zu machen, nur um das explizit hinzuschreiben. Ich habe die AI meines geringsten Misstrauens gefragt wie das Pattern heißt und sie sagt Parse, don't validate.

Fehlerbehandlung würde ich davon separat sehen. Ein paar unsortierte Links:
Robustness Principle
Let it crash
Fail Fast

Ich habe jetzt wenig Zeit das weiter auseinanderzudividieren, aber ich stimme Jonathan voll zu: wenn du für dich und dein Vergnügen programmierst, lass es crashen. Wenn du an einem System arbeitest dass nicht crashen darf weißt du normalerweise welche Garantien du geben willst und das weitere ergibt sich daraus. Im luftleeren Raum ist das alles akademisch.