3. Name eines Anti-Patterns gesucht

Name eines Anti-Patterns gesucht


  • @Arcoth
    Der Knackpunkt ist: was bedeutet "nicht erwartet"?
    Überleg dir einfach mal ein Beispiel: eine Funktion

    unique_ptr<Image> ExtractFrame(string const& videoFilePath, size_t frameNumber);

    Eine höchst unvollständige Liste von Dingen die dabei schief gehen können:
    * Das File existiert nicht
    * Das File existiert, kann aber nicht gelesen werden (z.B. weil von nem anderen Prozess mit "deny read" geöffnet)
    * Das File existiert, hat aber keines der unterstützten Container-Formate
    * Das File existiert, hat auch ein unterstütztes Container-Format, aber keinen Video-Stream
    * Das File existiert, hat auch ein unterstütztes Container-Format, aber der Video-Stream hat ein nicht unterstütztes Format
    * Das File existiert, ..., hat aber weniger Frames als frameNumber
    * Das File existiert, ..., aber irgendwo in dem ganzen Prozess ist der Speicher ausgegangen
    * Das File existiert, hat auch grundsätzliche ein gültiges Format, aber die Daten für Frame # frameNumber sind defekt (Prüfsummenfehler oder was weiss ich)
    uswusf.

    Jetzt kann man argumentieren: das sind alles Dinge die die Funktion erwarten muss, also alles "Normalfall", also nirgends Exceptions verwenden. Diese Argumentation halte ich für beknackt. Denn dann können wir Exceptions gleich einäschern -- es bleibt dann nämlich nichts mehr übrig wo wir wirklich Exceptions verwenden sollten. Ausser als Ersatz für Assertions. Nur dafür sind Exceptions eigentlich gar nicht so gut geeignet, denn wenn eine Assertion danebengeht, dann ist fraglich ob man das Programm überhaupt noch fortsetzen sollte.

    Man kann aber auch argumentieren: Es gibt hier genau einen "erwarteten" Fall, und zwar dass alles passt und die Funktion erfolgreich ein Frame extrahieren kann. Alles andere sind dann Ausnahmen, und wir verwenden dafür Exceptions. Diese Argumentation halte ich für viel sinnvoller.

    Je nachdem für was die Funktion genau eingesetzt wird können vielleicht noch weitere "normale" Fälle dazukommen. Das würde aber etwas zu weit führen, hier geht's ja nur um ein Beispiel.

    BTW: Theoretisch kann man Exceptions auch für beides verwenden, und die Fälle sogar unterscheiden. Die "Assertion-artigen" Fehler wären dann von std::logic_error abgeleitet, und alle anderen von std::runtime_error . Dummerweise gibt's aber viel zu viele Libraries die eigene Exception-Klassen verwenden, die entweder direkt von std::exception abgeleitet sind, oder gleich überhaupt gar keine Basisklasse haben. Dadurch ist es in der Praxis nicht möglich zu sagen "fang alle runtime Fehler" -- es sei denn man verwendet keine fremden C++ Libraries, und hat keinen eigenen Legacy-Code der einem dreinpfuschen würde.

    EDIT: Grammatik.

    0

  • huhasfuhduhuhu schrieb:

    Um mal den Erstsemester Talk hier zu unterbrechen

    Oh grosser grosser Meister, belehre uns!!! 🙄

    huhasfuhduhuhu schrieb:

    "Loop before you leap" (LBYL).

    Look before you leap.
    Ich kannte den Begriff nicht, aber dass "Loop before you leap" (offenbar ein Markenname für irgendwelche Damenschuhe) Quatsch ist war irgendwie offensichtlich.

    0
  • K
     

    unique_ptr<Image> ExtractFrame(string const& videoFilePath, size_t frameNumber);

    Das Problem mit Funktionen im God Mode ist ... der God Mode. Ich sach nur MMMMMMMonster kill.

    Ansonsten kann das Beispiel auch sehr gut mit Fehlercodes funktionieren. Ob irgendwo ein if (...) throw ... oder if (res != SUCCESS) return ... ist dabei unerheblich. Manchmal nutze ich auch:

    bool fun1(...);
bool fun2(...);

bool b = true;
b = b && fun1...
b = b && fun2...
return b;

    Der Unterschied ist semantischer Natur.

    0

  • knivil schrieb:

    unique_ptr<Image> ExtractFrame(string const& videoFilePath, size_t frameNumber);

    Das Problem mit Funktionen im God Mode ist ... der God Mode. Ich sach nur MMMMMMMonster kill.

    Was für ein sinnfreies Kommentar!

    Dir ist schon klar dass ich in keiner Weise auch nur angedeutet habe, dass diese Funktion als Monolith implementiert sein soll, ja?
    Aber nö, ist dir vermutlich nicht klar, weil du ja immer davon ausgehst dass was andere schreiben nur Mist sein kann.

    Alter Schwede!

    0
  • K

    Was für ein sinnfreies Kommentar!

    Mich zu verstehen ist schwer. Der Fehler liegt bei mir.

    Monolith implementiert sein soll

    Wie sie implementiert ist, ist doch egal. Die Funktion ist ein schlechtes Beispiel, da sie zu viel macht. Deswegen halte ich das Beispiel fuer ungeeignet.

    weil du ja immer davon ausgehst dass was andere schreiben nur Mist sein kann

    Unterstellung. Aber vielleicht hast du recht: ninety percent of everything is crap

    0

  • @knivil
    Geht's noch?

    Wenn ich genau diese Funktionalität in einem Programm brauche, dann schreib ich dafür genau so eine Funktion.
    Oder soll ich die 5-10 Zeilen die diese Funktion vermutlich haben wird dort reinschreiben, wo sonst der Funktionsaufruf stehen würde? Vielleicht auch noch an mehreren Stellen dupliziert? WTF?

    Argumentierst du jetzt für oder gegen Spaghetticode, entscheide dich!

    Davon abgesehen: dein Vorwurf "god mode" zeigt klar, dass du auf dem völlig falschen Dampfer bist. Mir ging es bei dem Beispiel gerade darum, eine Funktion zu wählen, die ganz klar sehr viele andere Klassen/Unterfunktionen benötigen wird. So dass die Tests auf die verschiedenen "Fehlerbedingungen" über viele viele Klassen/Funktionen/Schichten versteut sein werden. Weil dann auch klar wird, warum es Sinn macht dafür Exceptions zu verwenden.

    0

  • knivil schrieb:

    Ansonsten kann das Beispiel auch sehr gut mit Fehlercodes funktionieren. Ob irgendwo ein if (...) throw ... oder if (res != SUCCESS) return ... ist dabei unerheblich. Manchmal nutze ich auch:

    bool fun1(...);
bool fun2(...);

bool b = true;
b = b && fun1...
b = b && fun2...
return b;

    Der Unterschied ist semantischer Natur.

    Und wie mach' ich das wenn ich auch wissen will was nicht geklappt hat, nicht nur ob alles geklappt hat?

    0
  • K

    GetLastError, errno, Fehlercode fuer jeden Prozessschritt. Beispiel:

    enum e_video
{ e_success = 0
, e_load_failed = 1
, ...
};

int res = 0;
res = load_file(...)
if (res != e_success) return res;
res = read_header(...)
if (res != e_...

    Es sind wahrscheinlich genauso viele if s wie in einer throw Loesung. Als Spagethi-Code bezeichne ich das auch nicht. Klar, ordenlich aufgereiht, strukturiert. Einfach.

    Anmerkung: Wir sind 'Rund um die Programmierung', d.h. es geht nicht nur um Sprachen mit Exceptions. Und was fuer C gueltigkeit hat, muss noch lange nicht in C++ ungueltig sein. Von Lisp will ich gar nicht reden.

    0

  • knivil schrieb:

    Anmerkung: Wir sind 'Rund um die Programmierung', d.h. es geht nicht nur um Sprachen mit Exceptions. Und was fuer C gueltigkeit hat, muss noch lange nicht in C++ ungueltig sein. Von Lisp will ich gar nicht reden.

    Meine Antwort mit der bösen (angeblichen) "God-Mode Funktion", bezieht sich auf eine Frage von Arcoth bezüglich Exceptions. Und jetzt darf ich in der Antwort nicht voraussetzen dass es Exceptions gibt? Spinnst du jetzt komplett?

    0
  • K

    Entschuldige bitte. Da liegt mein Missverstaendnis. Erwartete Fehler sind ... subjektiv. Aber um trotzdem zu antworten: Genannte Loesung funktioniert auch in Anwesenheit von Exceptions.

    Spinnst du jetzt komplett?

    Jetzt krieg dich mal wieder ein. Kann ja nicht ahnen, dass du nie UT gespielt hast.

    0

  • @hustbaer: Vielen Dank, leuchtet ein.

    Ich lass' euch mal in Ruhe debattieren.

    0

  • knivil schrieb:

    Es sind wahrscheinlich genauso viele if s wie in einer throw Loesung.

    Nein. Es sind viel viel mehr if's. Muss ich dafür jetzt echt ein Beispiel liefern?

    Als Spagethi-Code bezeichne ich das auch nicht. Klar, ordenlich aufgereiht, strukturiert. Einfach.

    Darauf war das auch nicht bezogen. Du kannst nicht einfach ein Wort das ich in meiner Antwort geschrieben habe hernehmen, und auf irgendwas beziehen was dir grad einfällt. Also du kannst natürlich schon, aber es macht nicht sehr viel Sinn.

    Das war bezogen auf

    knivil schrieb:

    Wie sie implementiert ist, ist doch egal. Die Funktion ist ein schlechtes Beispiel, da sie zu viel macht. Deswegen halte ich das Beispiel fuer ungeeignet.

    Aber vielleicht hab ich dich da jetzt misverstanden. Ich hab das als "die Funktion tut zu viel, ganz egal wie sie implementiert ist" verstanden. Was mMn. totaler Unfug ist.

    Falls es dir wirklich nur um die Tauglichkeit als Beispiel geht: wieso ist das ein schlechtes Beispiel? Mir geht es gerade um Dinge wo verdammt viel schief gehen kann, weil die Funktion so viel tut (und so viele andere Funktionen/Klassen/... verwendet).

    Bilderbuchbeispiele sind meist trivial. Und triviale Beispiele sind ... naja eben trivial. Und damit meistens ziemlich uninteressant.

    0
  • K

    Am Ende geht's sowieso nur darum was besser ist: Welcher Fehler ist als Fehlercode umzusezten, als Exception, als leeres Object, boost::optional, Haskells Either, ... Ich frage mich, warum Fehlerbehandlung/Exceptions immer so ein Streitthema sind. Wahrscheinlich sind alle Beispiel auf mehrere Arten elegant, einfach und uebersichtlich zu loesen. Deswegen sind im Falle der Fehlerbehandlung alle Beispiele ungeeignet. Unter diesem Gesichtspunkt ziehe ich meine Kritik zurueck, weil wie du bereits sagtest: trivial.

    Ok, Variante b) hat mehr ifs. Variante a) bool + errno/GetLastError oder aequivalentes nicht, bzw. short circut Anwendung.

    0
  • Mod

    Willst du damit andeuten, eine gemischte Fehlerbehandlung wäre das Optimum? 😮
    Gerade so etwas wie errno geht ja wohl überhaupt nicht, besonders wenn man Exceptions zur Verfügung hätte.

    0
  • V

    knivil schrieb:

    Als Antipattern wuerde ich es nicht bezeichenen. Es gibt einige Anwendungsfaelle, beispielsweise:

    bool get_number(string_type str, double& d)
{
    if (check_number(str)) //eg. with a regular expression or is digit or ...
       d = retrieve_number(str);
    else
      return false;
    return true;
}

    Manche wuerden ja boost::lexical_cast benutzen. Ich wahrscheinlich nicht. Weils nur 'ne Zahl ist und weil ich vielleicht in C programmiere.

    Jo, klar. Da keiner an str rumfummeln kann, ist es kein "Fehler" mehr, vorher zu testen. Manchmal will man vorher testen (passt der C-Strings ins Ergebnis (strlen/if/strcpy) und manchmal nachher (gabs Übertrag bei der Addition?) und manchmal mittendrin(strncpy).

    Der Code ist doch nur ein Patternumsetzer damit Du ab jetzt statt

    if(check_number(str))
   d=retrieve_number(str);
else
   cerr<<"fehler!";

    das vermeintlich bessere

    if(get_number(str,d))
   //alles ok;
else
   cerr<<"fehler!";

    schreiben kannst.

    Also wenn Du obigen Code schreibst, dann bestätigst Du nicht, daß das kein Antipattern sei, sondern bestätigst, daß Du an höheren Stellen dieses Antipattern nicht verwenden willst.

    Hier mag man gerne nur eine Funktion haben statt zweien, damit die beiden gar nicht erst die Chance haben, auseinanderzulaufen, wie daß die checknumber 1e2 schluckt, die retrieve_number mindestens 1.e2 braucht. Und gerne ist die eine Funktion im "Normalfall", daß kein Fehler fliegt, auch schneller.

    Warum's bei Fehlerbehandlung Steit gibt? Hab den Eindruck, das kommt immer, wenn Du Exceptions schlechtredest.

    Und ich würde das return true dichter zum Erfolgsfalle ziehen

    bool get_number(string_type str, double& d)
{
    if (check_number(str)) //eg. with a regular expression or is digit or ...
    {
       d = retrieve_number(str);
       return true;
    }
    else
      return false;
}

    oder besser so einen Vortest nicht reinverschachteln lassen.

    bool get_number(string_type str, double& d)
{
    if (!check_number(str)) //eg. with a regular expression or is digit or ...
       return false;

    d = retrieve_number(str);
    return true;
}
    0

  • knivil schrieb:

    Ok, Variante b) hat mehr ifs. Variante a) bool + errno/GetLastError oder aequivalentes nicht, bzw. short circut Anwendung.

    Ja, Variante a) kann ähnlich kompakt ausfallen (wenn man die "&&" nicht mitzählt, und Funktionen wo man z.B. Cleanup-Code benötigt).

    Wenn man keinen Returncode braucht ist das vermutlich auch OK.

    Wenn man schon einen Returncode braucht, dann kann es doof werden.
    Weil der Cleanup-Code Aufrufe enthalten könnte die den "last error" überschreiben.
    Kann man natürlich wieder "fixen" indem man den Returncode über Output-Parameter statt globale macht.
    Alles in allem bleibt's aber umständlich.

    0
  • ?

    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.

    0

  • 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.

    0
  • ?

    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.

    0
  • T

    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));
  }
}
    0
Anmelden zum Antworten