Wie viel prüft ihr gültige Werte in Funktionen?



  • Hi,

    eine absolute Standardfrage, aber mir deucht, ich mache hier einiges suboptimal.

    Sagen wir, irgendeine meiner Klassen, die auch im Endeffekt nur ich nutze, sieht so aus:

    class NumberSets
    {
    public:
        typedef std::vector<std::set<int>> NumberSets;
    
        // ...
    
        int GetSumForSet(int index)
        {
            int sum = 0;
    
            for(int number : numberSets[index])
                sum += number;
    
            return sum;
        }
    
    private:
        // a vector of number sets
        NumberSets numberSets;
    };
    

    Irgendwo hörte ich mal, man solle nicht ständig alle Eingabewerte prüfen. Aber im echten Leben ruft man die Funktion dann mal mit dem falschen Index auf, weil man im UI den Index gecached hat aber kein richtiges Update gemacht hat (was auch immer!) und hat den Crash, der nicht nachzuvollziehen ist. 😞

    Prüft ihr hier also index out of bounds und falls ja, throwt ihr dann oder habt ihr nur den assert für den Debug-Fall drin und stellt ansonsten mit Tests sicher, dass hier schon keiner falsch aufruft?

    Viele Grüße
    Eisflamme



  • Früher habe ich dafür assert verwendet, aber vor einigen Jahren bin ich zu der Überzeugung gekommen, dass das der falsche Weg ist. Inzwischen werfe ich auch bei internen Logikfehlern nur noch Exceptions und versuche, möglichst viel Zusatzinformation in der Exception-Message unterzubringen.

    Gerade bei einer Serveranwendung ist es stressig, wenn sie irgendwann mit einem nichtssagenden SIGSEGV abstürzt, man dann eine Debugvariante in den (Live- !)Betrieb nimmt und dann noch beten muss, dass das Problem in den nächsten Tagen oder Wochen wieder auftritt.
    Habe ich dagegen eine aussagekräftigere Exception, reicht das in der überwiegenden Zahl der Fälle aus, um das Problem ohne zusätzliches Debugging einzukreisen und zu beheben.

    Desweiteren kann man eine Exception eben auch beliebig fangen - besonders dann nützlich, wenn man weniger kritische Subtasks ausführen möchte, die selbst im Falle eines internen Fehlers im Ablauf nicht den Rest der Anwendung in den Tod reißen dürfen. Solche Subtasks müsste man andernfalls von externen Prozessen erledigen lassen.

    Ich denke, man tut allgemein gut daran, seinen Programmierstil so anzupassen, dass es auch im Falle von Programmierfehlern möglichst nie zu einem unkontrollierten Programmabsturz kommt (bzw. allgemein zu undefiniertem Verhalten).



  • ;qwerty schrieb:

    Ich denke, man tut allgemein gut daran, seinen Programmierstil so anzupassen, dass es auch im Falle von Programmierfehlern möglichst nie zu einem unkontrollierten Programmabsturz kommt (bzw. allgemein zu undefiniertem Verhalten).

    Denke ich auch.

    Es gibt aber noch die Alternative: Assertions die auch im Release Build drinnen bleiben. Und dann im Falle des Falles den Prozess kontrolliert abschiessen. Davor kann man noch schön loggen was man loggen möchte und wenn man will auch nen Crashdump schreiben.

    Und natürlich kann man sein OS so einstellen dass für crashende Programme automatisch ein Crashdump geschrieben wird. Und wenn man bei jedem Deploy-Vorgang den Code taggt und die Symbol-Files aufhebt, kann man mit solchen Crashdumps auch 'was anfangen. (Wir verwenden da seit einiger Zeit den Microsoft "Source Server". D.h. wir instrumentieren unsere PDB Files mit dem Pfad zu den getaggten Source-Files, und stecken die EXE, DLL und PDB Files in einen Symbol-Server. Beim Öffnen eines Crashdumps holt sich Visual Studio dann automatisch die passenden EXE/DLL/PDB Files aus dem Symbol-Server und dann den Source mit dem das Zeug gebaut wurde aus dem Version Control System.)

    Natürlich haben beiden Lösungen Vor- und Nachteile.

    Bei Exceptions kann es passieren dass man Probleme mit Fehlerbehandlungscode bekommt, der nicht auf bestimmte Logikfehler ausgelegt ist, und daher in solchen Fällen Mist baut. Oder auch gerne Code der bei bestimmten Dingen mit "kann eh nicht schiefgehen" rechnet, und daher beim Unwinding aus der Mitte eines mutmasslichen "noexcept" Abschnittes heraus Invarianten zerstört.

    Und beim Prozess-Abbrechen kann es passieren dass der Server so oft beendet wird (und natürlich automatisch neu gestartet - sonst hätte man sowieso ein grosses Problem), dass die Anwendung dadurch unverwendbar wird. z.B. wenn Clients den Bug mit bestimmten Requests triggern, auf die Requests natürlich keine Antwort erhalten, und deswegen Retries machen. Und das so lange bis eine Antwort daherkommt. Die natürlich nie kommt wenn der Bug deterministisch ist und durch bestimmte Requests garantiert getriggert wird.

    Was in der Praxis häufiger Probleme verursachen wird, kann ich nicht einschätzen. Persönlich verwende ich hauptsächlich Exceptions, wobei ich sicher nicht so viel Checks einbaue wie man einbauen könnte.
    Bei bestimmten Dingen gehe ich einfach davon aus dass die Parameter OK sind.
    Bei anderen Dingen mache ich allerdings grundsätzlich Checks. z.B. würde ich nie den Returnwert von map<>::find verwenden ohne vorher gegen map<>::end zu vergleichen.
    Und Dinge die sehr zuverlässig zu nem Crash führen (z.B. Dereferenzieren von Nullpointern) prüfe ich auch nicht. Dafür gibt es die erwähnten Crashdumps.


  • Mod

    Prüfen auf Logikfehler zur Laufzeit? Nein, nie. Wozu programmiere ich denn in C++, bloß um dann die Kindersicherung einzuschalten? Dann habe ich die falsche Sprache für das Projekt gewählt.

    Benutzereingaben werden vor der Verarbeitung genauestens geprüft, aber nicht die Tatsache, ob der Programmierer in der Lage ist, ein korrektes Programm zu schreiben.



  • @Eisflamme
    Konkret zu deinem Beispiel...

    Wenn es keinen Sinn macht, zu erlauben dass eine Funktion mit einem "out of range" Parameter aufgerufen wird, dann sollte mMn. zumindest eine Assertion rein. Damit es beim Testen/Debuggen nicht übersehen werden kann.

    Weiters kann man dann...
    * Das Programm abbrechen oder
    * Eine Exception werfen oder
    * Die Funktion einen "sicheren" Wert zurückgeben lassen (in deinem Beispiel könnte das vielleicht 0 sein)

    Zusätzlich kann man noch...
    * Einen Logfile-Eintrag schreiben und/oder
    * Einen Crashdump schreiben

    Um dabei Code-Duplizierung zu vermeiden kann man sich dafür z.B. schöne Makros machen.
    Also z.B. ein ASSERT_AND_LOG(expression) , ASSERT_AND_THROW(expression) oder ASSERT_AND_ABORT(expression) Makro.

    Oder halt auch, wie manche es für richtig und gut halten, nix machen.

    Ich bin kein Fan von "nix machen".

    Welches aus abort/throw/return-safe-value die beste Wahl ist, ... da muss ich leider sagen: kann ich dir nicht sagen 🙂
    Und hängt auch stark davon ab wie man entwickelt/testet. Bei Projekten mit massig automatisierten Tests mit hoher Path-Coverage kann "Programm abbrechen" ganz toll funktionieren. Bei Projekten die eher nach dem Motto "it compiles => ship it" entwickelt werden, funktioniert "Programm abbrechen" weniger gut, und das Werfen von Exceptions bzw. das Zurückgeben von "sicheren Werten" wird auf einmal viel attraktiver.



  • SeppJ schrieb:

    Prüfen auf Logikfehler zur Laufzeit? Nein, nie. Wozu programmiere ich denn in C++, bloß um dann die Kindersicherung einzuschalten? Dann habe ich die falsche Sprache für das Projekt gewählt.

    Wieso sollte C++ die falsche Sprache sein?
    Und welche Sprachen wären deiner Meinung nach besser geeignet?

    SeppJ schrieb:

    Benutzereingaben werden vor der Verarbeitung genauestens geprüft, aber nicht die Tatsache, ob der Programmierer in der Lage ist, ein korrektes Programm zu schreiben.

    Ob der Programmierer in der Lage ist ein korrektes Programm zu schreiben muss man nicht prüfen, da die Antwort bei realen Projekten mit realen Programmierern und nicht-trivialem Umfang bekanntermassen "Nein" ist.


  • Mod

    hustbaer schrieb:

    SeppJ schrieb:

    Prüfen auf Logikfehler zur Laufzeit? Nein, nie. Wozu programmiere ich denn in C++, bloß um dann die Kindersicherung einzuschalten? Dann habe ich die falsche Sprache für das Projekt gewählt.

    Wieso sollte C++ die falsche Sprache sein?
    Und welche Sprachen wären deiner Meinung nach besser geeignet?

    Die Chance ist hoch, dass ich C++ für Projekte wähle, bei denen ich eben keine überflüssigen Prüfungen aufgebrummt bekommen will. Wenn ich aber ohnehin überflüssige Überprüfungen einprogrammiere, dann kann ich auch gleich eine Sprache wählen, bei der dies schon mit eingebaut ist. Das ist bei so ziemlich allen weniger maschinennahen Sprachen der Fall.

    SeppJ schrieb:

    Benutzereingaben werden vor der Verarbeitung genauestens geprüft, aber nicht die Tatsache, ob der Programmierer in der Lage ist, ein korrektes Programm zu schreiben.

    Ob der Programmierer in der Lage ist ein korrektes Programm zu schreiben muss man nicht prüfen, da die Antwort bei realen Projekten mit realen Programmierern und nicht-trivialem Umfang bekanntermassen "Nein" ist.

    Dann sind wir uns ja einig, dass man das nicht prüfen braucht.

    (Es ist übrigens recht frech zu implizieren, dass nur die Sachen, die du so machst, "real" und "nicht-trivial" wären. Bloß weil die Sachen, die ich typischerweise mache, von 1-3 Leuten programmiert werden können, heißt das nicht, dass diese weniger trivial oder real wären. Wir denken wahrscheinlich sogar sehr viel mehr darüber nach als der Durchschnittsprogrammierer an umfangreicheren Projekten und es gibt ganz real noch sehr viel mehr Kleingruppen. Programmierung ist ein größeres Feld als nur an irgendwelchen monolithischen Businessanwendungen mit 5000 Entwicklern und 200 Millionen Codezeilen zu werkeln. In manchen Teilen davon machen solche Prüfungen Sinn, in anderen überhaupt nicht.)



  • SeppJ schrieb:

    Prüfen auf Logikfehler zur Laufzeit? Nein, nie. Wozu programmiere ich denn in C++, bloß um dann die Kindersicherung einzuschalten? Dann habe ich die falsche Sprache für das Projekt gewählt.

    Benutzereingaben werden vor der Verarbeitung genauestens geprüft, aber nicht die Tatsache, ob der Programmierer in der Lage ist, ein korrektes Programm zu schreiben.

    Für viele Programme, die in C++ geschrieben werden ist das ein berechtigter Einwand, es gibt allerdings auch Einsatzgebiete,
    die eine besonders hohe Programmstabilität erfordern und wo man selbst bei einem programmierfehler nicht kommentarlos aussteigen sollte:
    z.B. für medizinische Geräte, besonders wenn diese Patienten überwachen oder Ärzte bei Eigngriffen unterstützen, Steuersoftware für technische Anlagen
    oder Fahrzeuge jeglicher Art (Flugzeuge, Autos, oder ... Ariane 5 anyone? ;))

    Diese Programme sollten wohl zumindest würdevoll mit so etwas wie einem "Not-Aus" beendet werden,
    bzw. sich umgehend selbst wieder in einen wohldefinierten Zustand versetzen.
    Ansonsten bin ich allerdings auch ein Freund von ausgiebigem Testen mit vielen Assertions anstatt Taktzyklen für Tests zu verschwenden,
    die bei korrekter Programmierung nicht negativ ausfallen können.

    Gruss,
    Finn


  • Mod

    Finnegan schrieb:

    SeppJ schrieb:

    Prüfen auf Logikfehler zur Laufzeit? Nein, nie. Wozu programmiere ich denn in C++, bloß um dann die Kindersicherung einzuschalten? Dann habe ich die falsche Sprache für das Projekt gewählt.

    Benutzereingaben werden vor der Verarbeitung genauestens geprüft, aber nicht die Tatsache, ob der Programmierer in der Lage ist, ein korrektes Programm zu schreiben.

    Für viele Programme, die in C++ geschrieben werden ist das ein berechtigter Einwand, es gibt allerdings auch Einsatzgebiete,
    die eine besonders hohe Programmstabilität erfordern und wo man selbst bei einem programmierfehler nicht kommentarlos aussteigen sollte:
    z.B. für medizinische Geräte, besonders wenn diese Patienten überwachen oder Ärzte bei Eigngriffen unterstützen, Steuersoftware für technische Anlagen
    oder Fahrzeuge jeglicher Art (Flugzeuge, Autos, oder ... Ariane 5 anyone? ;))

    Es wurde ja danach gefragt, was wir machen, nicht was wir uns vorstellen können. Wie gesagt, wäre der hier geschilderte Fall einer, wo ich niemals prüfen würde und erst recht nicht aus den geschilderten Gründen. Aber ich programmiere auch keine Steuerungen für medizinische Geräte.



  • SeppJ schrieb:

    Die Chance ist hoch, dass ich C++ für Projekte wähle, bei denen ich eben keine überflüssigen Prüfungen aufgebrummt bekommen will. Wenn ich aber ohnehin überflüssige Überprüfungen einprogrammiere, dann kann ich auch gleich eine Sprache wählen, bei der dies schon mit eingebaut ist. Das ist bei so ziemlich allen weniger maschinennahen Sprachen der Fall.

    Das impliziert dass C++, bei Projekten, wo der Grossteil des Codes nicht absolut performancekritisch ist, grundsätzlich "die falsche Sprache" ist. Ich sehe das anders. Ich denke C++ hat auch andere wichtige Vorteile gegenüber anderen Sprachen. Wie z.B. deterministische Finalisierung.

    ps:

    SeppJ schrieb:

    Wie gesagt, wäre der hier geschilderte Fall einer, wo ich niemals prüfen würde und erst recht nicht aus den geschilderten Gründen.

    Auch kein assert() ?



  • SeppJ schrieb:

    Es wurde ja danach gefragt, was wir machen, nicht was wir uns vorstellen können. Wie gesagt, wäre der hier geschilderte Fall einer, wo ich niemals prüfen würde und erst recht nicht aus den geschilderten Gründen. Aber ich programmiere auch keine Steuerungen für medizinische Geräte.

    Jo, klang ein wenig so als wäre das eine allgemeine Aussage zu C++.
    Bei dem gegebenen Beispiel von Eisflamme muss ich allerdings sagen,
    dass ich mich, ohne den Kontext zu kennen, nur schwer zurückhalten
    kann in dem Fall 0 zurückzugeben. Das springt einen förmlich an 🙂
    Summe von nichts ist 0 - damit könnte ich gut leben... führt nachher
    auch zu wenig Widersprüchen, wenn man anfängt Summen von Summen zu bilden.

    Finnegan


  • Mod

    Finnegan schrieb:

    Jo, klang ein wenig so als wäre das eine allgemeine Aussage zu C++.
    Bei dem gegebenen Beispiel von Eisflamme muss ich allerdings sagen,
    dass ich mich, ohne den Kontext zu kennen, nur schwer zurückhalten
    kann in dem Fall 0 zurückzugeben. Das springt einen förmlich an 🙂
    Summe von nichts ist 0 - damit könnte ich gut leben... führt nachher
    auch zu wenig Widersprüchen, wenn man anfängt Summen von Summen zu bilden.

    Ich hasse 0 als Rückgabewert für Fehler. Das macht nur Sinn, wenn 0 kein gültiger Wert sein kann. Aber 0 kann sehr realistisch die Summe einer Gruppe von Zahlen sein. Das führt zu dem gleichen Problem, weswegen man atoi nicht vernünftig benutzen kann. Nicht nur dass man den Fehlerstatus prüfen muss, man kann ihn nicht einmal sicher erkennen!

    Abgesehen davon, ist ein Fehlerstatus im Rückgabewert allgemein fast immer doof. Man muss den Code mit lauter

    if (funktion(blah) == fehlerstatus)
    {
      fehlerbehandlung;
      hier;
      und;
      jetzt;
    }
    

    verschandeln. Exceptions wurden aus gutem Grund erfunden!



  • @SeppJ
    Die zurückgegebene Null wäre hier ja kein "Rückgabewert für Fehler", sondern einfach ein Wert, von dem man annimmt, dass das restliche Programm damit weitermachen kann, ohne schlimmen Unsinn zu bauen.
    Um zu wissen ob das eine sinnvolle Annahme ist, müsste man natürlich etwas mehr über das Programm wissen.

    Quasi so wie man aus einer GetTranslatedText(string id, string language) Funktion, in Fällen wo id nicht gefunden wird, oft einfach "Text '" + id + "' not found!" zurückgeben kann.


  • Mod

    Dann sollte es etwas auffälligeres sein. 0xDEADEAD oder 0xFFFFFFFF oder ähnlich. Dann sieht man, wie beim "Text not found", dass etwas falsch lief.

    Viele Edits: Irgendwie habe ich es heute nicht so mit Rechtschreibung.



  • Hi,

    danke für die Diskussion!

    Also Crashdumps lasse ich mir auch erzeugen wie vorgeschlagen. pdb-Dateien pro Release merke ich mir und ich nutze CrashRprt, das mir automatisch den Dump zukommen lässt.

    Exceptions nutze ich zurzeit leider viel zu wenig, ich gehe oft davon aus, dass alles einfach funktioniert und ich das "schon richtig gemacht hab". Das stimmt auch fast immer, aber dann kommt halt doch mal irgendwo ein Fehler, der zwar ge-crash-dumpt wird, aber dann bringt mir der Crashreport manchmal nichts, weil der Fehler in nt.dll oder so steckt. 😞 Da steckt er natürlich nicht, wird nur so angezeigt.

    Da frage ich mich halt, was ich verbessern kann, um sicher zu gehen später auch alle Crashs rauszukriegen. Logging wäre sicher eine Option, da manche Kunden regelmäßig Crashs miterleben und andere wiederum nie. Das ist sehr lästig. Denen könnte ich anbieten mal mit Logging zu starten oder so was...



  • Hier sind noch zwei Videos von John Lakos:
    https://channel9.msdn.com/Events/CPP/C-PP-Con-2014/Defensive-Programming-Done-Right-Part-I
    https://channel9.msdn.com/Events/CPP/C-PP-Con-2014/Defensive-Programming-Done-Right-Part-II

    Soweit ich mich erinnere, phi­lo­so­phie­rt er auch über das Thema dieser Diskussion.



  • SeppJ schrieb:

    Ich hasse 0 als Rückgabewert für Fehler. Das macht nur Sinn, wenn 0 kein gültiger Wert sein kann. Aber 0 kann sehr realistisch die Summe einer Gruppe von Zahlen sein. Das führt zu dem gleichen Problem, weswegen man atoi nicht vernünftig benutzen kann. Nicht nur dass man den Fehlerstatus prüfen muss, man kann ihn nicht einmal sicher erkennen!

    Ja, das sehe ich auch so, aber er Grund warum ich hier 0 zurückgeben würde ist nicht, dass 0 ein Fehlerstatus ist, sondern eben ein gültiger und sinnvoller Wert.
    Hier wird schließlich die Summe von Zahlen in einer Menge gebildet, und man kann durchaus argumentieren dass die Summe aller Zahlen in der leeren Menge 0 ist, bzw. dass die Summe aller Zahlen in einer nicht existierenden Menge ebenfalls 0 ist.
    In diesem Fall hätte die Funktion einfach keinen Fehler auf den sie laufen könnte. Das funktioniert aber nur, weil eben 0 ein gültiger Wert ist, der zu keinen logischen Widersprüchen führt. Hätte ich z.B. eine Funktion, die mir die dritte Zahl im fünften Set zurückgeben soll würde ich selbstverständlich auch einen Fehler werfen wenn diese nicht existiert.

    Finnegan


Anmelden zum Antworten