Name eines Anti-Patterns gesucht



  • hustbaer schrieb:

    Bzw. schreibst du sogar ein paar Dinge die mMn. sogar höchst fragwürdig sind. Ich verstehe z.B. nicht warum es schlimm ist, wenn bei fehlerhaften Input-Files irgendwo ne Exception fliegt. Ich meine, dafür sind Exceptions ja schliesslich da - um Ausnahmefälle, wie fehlerhafte Input-Files, zu behandeln.

    Vielleicht reden wir aneinander vorbei. Bei fehlerhaften Files darf gerne eine Exception fliegen. In diesem Fall wurde jedoch quasi per Exception-Mechanismus geprüft ob ein String eine gültige Darstellung einer Gleitkommazahl ist - wenn man mit ungültigen Daten in der Eingabe rechnen muss, sollte man IMHO keine Exceptions dazu verwenden. Eben weil dann z.B. der Debugger regelmäßig anhält, da es sich anscheinend wohl doch nicht um eine "Ausnahme" handelt.

    Wie dem auch sei, belehren wollte ich Dich jedenfalls nicht. Tut mir leid wenn das so rüber kam.



  • Morle schrieb:

    Bei fehlerhaften Files darf gerne eine Exception fliegen. (...) wenn man mit ungültigen Daten in der Eingabe rechnen muss, sollte man IMHO keine Exceptions dazu verwenden.

    Das widerspricht sich mMn.
    => Ich weiss nicht was du jetzt meinst. Ist es jetzt OK, oder nicht?

    Was ich nicht OK finde, ist sowas:

    bool IsFloat(string str)
    {
        try
        {
            float.Parse(str);
            return true;
        }
        catch // egal jetzt ob catch-all oder der spezielle Typ der bei unpassender Eingabe geworfen wird,
              // darum geht's mir hier nicht
        {
            return false;
        }
    }
    

    Das ist klarer Misbrauch.

    Wenn ich aber ein File verarbeite, wo halt irgendwo ein Float stehen muss wenn das File gültig ist, und ich will den lesen... dann finde ich es schon OK wenn ich den einfach lese, und im Fehlerfall halt ne Exception fliegt. Auch wenn es Datenfiles/Dokumente/Einstellungsfiles/... sind, die der User bearbeiten kann.

    Wichtig bei sowas ist natürlich dass es ne möglichst gute Fehlermeldung gibt, und dass das Programm nicht crasht (oder sonst wie Blödsinn baut).

    Man kann natürlich in einem Unternehmen oder für ein spezielles Projekt eigene Regeln aufstellen. Wie z.B. dass Input-Validierung "Exception-frei" zu passieren hat. (Und natürlich auch der restliche "normale" Betrieb der Anwendung.) Dann kann man beim Testen immer "break on every exception" eingestellt haben, und sobald eine fliegt kann man sicher sein dass es sich auszahlt zu gucken wo und warum die gefolgen ist.

    Mir persönlich wäre diese Regel aber etwas zu strikt. Weil man sich dadurch an sehr vielen Stellen unnötig Arbeit macht, und nicht wirklich so viel Arbeit beim Testen spart.



  • hustbaer schrieb:

    Morle schrieb:

    Bei fehlerhaften Files darf gerne eine Exception fliegen. (...) wenn man mit ungültigen Daten in der Eingabe rechnen muss, sollte man IMHO keine Exceptions dazu verwenden.

    Das widerspricht sich mMn.
    => Ich weiss nicht was du jetzt meinst. Ist es jetzt OK, oder nicht?

    Was ich nicht OK finde, ist sowas:

    bool IsFloat(string str)
    {
        try
        {
            float.Parse(str);
            return true;
        }
        catch // egal jetzt ob catch-all oder der spezielle Typ der bei unpassender Eingabe geworfen wird,
              // darum geht's mir hier nicht
        {
            return false;
        }
    }
    

    Das ist klarer Misbrauch.

    Full ACK. Das ist genau das Beispiel dafür, wo Exceptions missbraucht werden um eine Eigenschaft zu erkennen.

    Was ich im Sinn hatte war, wenn eine Eingabe validiert wird und ich der Meinung bin der Benutzer gibt in (übertrieben) 90% der Fälle kein Float ein, wo man aber eins erwartet, nicht einfach:

    float fEingabe = float.fromString(stringEingabe);
    

    zu machen, sondern eher

    float fEingabe;
    if (IsFloat(stringEingabe))
    {
     fEingabe =  float.fromString(stringEingabe);
    }
    else
    {
     // Fehlerbehandlung, z.B. eigene Exception werfen
    }
    

    zu schreiben. Die Implementierung von IsFloat wäre *nicht* - wie in deine Beispiel - über "versteckte" Exceptions.

    Noch eine Variante:

    try
    {
     fEingabe = float.fromString(stringEingabe);
    }
    catch (FloatToStringException e)
    {
     // Fehlerbehandlung, z.B. eigene Exception werfen
    }
    

    Die mittlere Lösung finde ich am praktischsten - es treten z.B. beim Debuggen nicht so viele (störende) Exceptions auf für Fehler, an die man eh gedacht hat. Möglicherweise auch gar keine, wenn man im ELSE Zweig keine Exception wirft sondern die Fehlerbehandlung über einen Rückgabewert gelöst hat. Wenn ich damit rechne 90% sind Fehleingaben, führt dies IMHO schon die Begrifflichkeit "Ausnahme" ad absurdum.
    Die Letzte ist sicherlich die eleganteste von den Beispielen.

    Viel wichtiger ist mir in diesem Zusammenhang aber noch, dass man bei Beispiel 1 und 2 sofort sieht, der Programmierer hat sich über die Fehlerquellen Gedanken gemacht und vertraut nicht darauf, dass Exceptions "schon irgendwo" gefangen werden.



  • In C# sind für so etwas Nullables zu verwenden.

    float? f = ParseFloat(str);
    

    In C++ wäre das boost::optional .

    boost::optional<float> f = parse_float(str);
    

    Aber das ist natürlich zu modern, zu allgemein verwendbar, zu fehlerunanfällig und zu performant für die meisten!

    Das eigentliche Anti-Pattern hier ist aber die String-Typisierung. Alles was irgendwie vom Benutzer kommt, wird viel zu lange als String durch die Gegend gereicht. Dann wird an zu vielen Stellen versucht den zu konvertieren.

    Die Darstellung von float als String gehört ganz weit weggekapselt, zum Beispiel ungefähr so:

    var variable = new MyGuiFramework.Variable<float>(0.0f, x => (x >= 0.0f), "Positive numbers only");
    var editField = new MyGuiFramework.Editor(variable);
    ..
    void OnSubmit()
    {
      float alreadyValidatedInput = variable.GetCurrentValue();
    }
    
    void OnLoad()
    {
      try {
        LoadConfigurationFile("config.json", new Dictionary<String, MyGuiFramework.AbstractVariable> {
          {"var1", variable}
        });
      }
      catch (MisconfigurationException e)
      {
        e.Errors.ForEach(err => PushErrorMessage(err.HumanReadableDescription));
      }
    }
    


  • TyRoXx schrieb:

    In C# sind für so etwas Nullables zu verwenden.

    float? f = ParseFloat(str);
    

    Ach sind sie das?
    Mit der Aussage wäre ich vorsichtig. Bei C# Nullables gibt's wohl einige Gotchas.
    Und das Framework verwendet da das TryDo(..., out param) Muster (z.B. float.TryParse , Dictionary.TryGetValue etc.). Mir ist klar dass der Grund nicht war dass Nullables problematisch sind, sondern dass das Framework CLI compliant sein muss. Evtl. auch daran, dass es noch gar keine Nullables gab wie das Framework entworfen wurde (wobei ich nicht glaube dass die Existenz von C# Nullables damals etwas geändert hätte).

    Trotzdem ist es ein sehr bekanntes Muster, einfach weil es das Framework so macht.
    Und da C# den grössten Nachteil den Output-Parameter in C++ haben (=Unsichtbarkeit) vermeidet, finde ich es auch nicht schlimm.

    Und den Prefix "Try" für den Funktionsnamen würde ich auf jeden Fall haben wollen. Den wegzulassen halte ich für schon fast grob fahrlässig. Wenn ich "parse" schreibe erwarte ich mir dass geparsed wird, genau so wie ich mir erwarte dass ein File aufgemacht wird wenn ich File.Open schreibe. Und nicht dass es bloss versucht wird.

    Aber das driftet jetzt schon wieder recht weit in Richtung OT 🙂

    TyRoXx schrieb:

    Das eigentliche Anti-Pattern hier ist aber die String-Typisierung. Alles was irgendwie vom Benutzer kommt, wird viel zu lange als String durch die Gegend gereicht. Dann wird an zu vielen Stellen versucht den zu konvertieren.

    Es ist schön wenn man Data-Binding einsetzen kann. Geht aber nicht immer. D.h. manchmal muss man damit leben. Weil ne bestehende Anwendung betreut/erweitert wird, und man nicht Zeit hat die ganze GUI neu zu machen. Weil man für ne Plattform entwickelt wo es keine brauchbare GUI Lib' mit Data-Binding gibt. Oder weil man ein Entwickler-Team hat das gut und effizient mit GUI Toolkit X arbeiten kann, und gerade keine Zeit/Geld investieren um die alle auf GUI Toolkit Y umzuschulen.



  • Morle schrieb:

    Was ich im Sinn hatte war, wenn eine Eingabe validiert wird und ich der Meinung bin der Benutzer gibt in (übertrieben) 90% der Fälle kein Float ein, wo man aber eins erwartet, nicht einfach: (...)

    Ich sag mal: wenn der Benutzer auch nur in 50% der Fälle was eingibt, was kein Float ist, wo ein Float sein sollte, dann hat die Anwendung eh ein anderes Problem 🙂

    Mir fällt jetzt auch kein Beispiel ein, wo es um mehr als Validierung geht, wo ich es "OK" fände, wenn die User in mehr als 50% der Fälle was unpassendes eingeben. Nen String in einen Float parsen ist ja "mehr als Validierung". Ein Beispiel mit "nur Validierung" wäre die Prüfung der minimalen Komplexität von Passwörtern. User sind faul, und wenn die minimale Komplexität recht hoch eingestellt ist, dann kanns leicht sein dass man 90% Fails hat.

    Wobei ich es auch da nicht als so besonders störend empfinden würde. Der User merkt ja nichts davon dass da ne Exception fliegt, und sie wird ja auch nicht geloggt. Weil der Programmierer schlau genug war hier speziell die FormatException zu fangen, und nur die, und die dann eben nicht zu loggen.

    Der einzige der was davon mitbekommt ist der Tester (oder der Entwickler beim Testen). Und der sollte mMn. schlau genug sein bei seinem Test hier 1x auf "Continue" zu drücken, wenn hier wieder das FormatException Fenster aufpoppt.



  • Patternsucher schrieb:

    Zwischen der Prüfung auf die Existenz der Datei und dem Löschen könnte ja theoretisch das Betriebssystem die Datei löschen und es würde in manchen Sprachen in diesem Fall eine Exception fliegen.

    Derartige Bugs werden oft mit "Time of check to time of use"-Bugs (TOCTTOU) bezeichnet.

    Das Spektrum geht in der Tat von einfachen "wenn exists(file), dann open(file)"-Sequenzen (was zumindest unter Unix eigentlich immer falsch ist) bis hin zum gezielten Angriff von privilegierten Prozessen, die möglicherweise derartige Bugs beim Umbennen oder Schreiben von Dateien besitzen.



  • Ich tendiere in letzter Zeit immer mehr, Exceptions zugunsten von Return Codes fallen zu lassen. Faelle, in denen ich noch Exceptions verwende: Ein TCP-acceptor Socket kann nicht alloziert werden, Speicher aus, etc, also fatal Errors. Die fange ich dann aber auch nicht und lasse mein Programm abschmieren.


Anmelden zum Antworten