About a code. Code Style Frage.



  • killerargument schrieb:

    Ausserdem sind Default-Argumente sehr oft ein Zeichen für Design-Fehler.

    Wenn du meinst... Nach meiner Erfahrung sind sie kein Designfehler, sofern die Parameter sinnvolle Vorgabewerte haben. Wieso sollte man 2 Funktionen für exakt die gleiche Funktionalität schreiben, wenn diese sich nur um ein Detail unterscheiden: Und zwar ob man mit einem festen Wert oder einem vorgegebenen arbeitet.

    Es hat durchaus seinen Grund warum viele Sprachen die Anfang die Defaultparameter nicht unterstützt hatten, diese nachträglich hinzugefügt haben.

    Nehmen wir mal ein ganz banales Beispiel: Fast an allen Stellen sollen Ausgaben auf 2 Nachkommastellen gerundet werden, an ganz wenigen Stellen benötigt man aber eine davon abweichende Anzahl. Warum kein Defaultparameter für die Anzahl der Nachkommastellen (Standard: 2)?



  • asc schrieb:

    Nehmen wir mal ein ganz banales Beispiel: Fast an allen Stellen sollen Ausgaben auf 2 Nachkommastellen gerundet werden, an ganz wenigen Stellen benötigt man aber eine davon abweichende Anzahl. Warum kein Defaultparameter für die Anzahl der Nachkommastellen (Standard: 2)?

    Das ist so ein typischer Design-Fehler.

    Da gehört entweder eine Konstante definiert, die an "fast allen Stellen" übergeben wird. Die Konstante ist dann vernünftig benannt und kommentiert, sodass man die Intention erkennen kann.

    Oder man erstellt zwei Funktionen mit unterschiedlichen Namen. Eine von denen hat dann das "Standardverhalten".



  • TyRoXx schrieb:

    asc schrieb:

    Nehmen wir mal ein ganz banales Beispiel: Fast an allen Stellen sollen Ausgaben auf 2 Nachkommastellen gerundet werden, an ganz wenigen Stellen benötigt man aber eine davon abweichende Anzahl. Warum kein Defaultparameter für die Anzahl der Nachkommastellen (Standard: 2)?

    Das ist so ein typischer Design-Fehler.

    Ich habe gelernt das Codeduplikate und unnötiger Code Designfehler sind.

    Warum soll ich 2 Funktionen pflegen, selbst wenn eine davon die andere Aufruft? Warum sollte ich die Funktionen unterschiedlich nennen, wenn sie grundsätzlich die gleiche Aufgabe hat (nur das in einem Fall eine weitere Angabe nötig ist)?

    Es ist ja nicht so das man hier eine Funktion mit einer großen Parameteranzahl hätte (was wiederum schlechtes Design ist). Wer definiert eigentlich das es sich hier um einen Design-Fehler handelt.

    TyRoXx schrieb:

    Da gehört entweder eine Konstante definiert, die an "fast allen Stellen" übergeben wird. Die Konstante ist dann vernünftig benannt und kommentiert, sodass man die Intention erkennen kann.

    Ich halte benannte Konstanten zwar grundsätzlich für etwas gutes, in dem Fall wäre die unnötige Codemenge aber weit höher. Ebenso wie es nicht intuitiv wäre die Funktion die sich nur um diesen einen Parameter unterscheidet anders zu nennen.


  • Mod

    Offensichtlich gibt es keine Intention die Kopie der Parameter zu verhindern. Der einzige Grund für eine Referenz könnte sein dass man e.g. die Adressen der Parameter speichert, oder eine Referenz darauf.

    Warum soll ich 2 Funktionen pflegen, selbst wenn eine davon die andere Aufruft?

    Du pflegst grundsätzlich nur eine. Die andere kann sogar perfect forwarding machen und nur ein Argument hinten dranhängen.



  • asc schrieb:

    Ich habe gelernt das Codeduplikate und unnötiger Code Designfehler sind.

    Warum soll ich 2 Funktionen pflegen, selbst wenn eine davon die andere Aufruft? Warum sollte ich die Funktionen unterschiedlich nennen, wenn sie grundsätzlich die gleiche Aufgabe hat (nur das in einem Fall eine weitere Angabe nötig ist)?

    Es gibt viele dos und donts in der Entwicklung. Dummerweise verwechseln manche Zeitgenossen ¨should not¨ mit ¨must not¨. Zweites Problem, manche donts schliessen sich gegenseitig aus. Hier ein schönes Beispiel default parameter und code duplikate.

    Ich meine, wenn beim Aufruf einer Funktion, der übergebene Wert für das Verständnis unerheblich ist, darf er durch einen Defaultparameter ersetzt werden. Sonst nicht.

    Mfg Martin



  • mgaeckler schrieb:

    Ich meine, wenn beim Aufruf einer Funktion, der übergebene Wert für das Verständnis unerheblich ist, darf er durch einen Defaultparameter ersetzt werden. Sonst nicht.

    Wenn der Wert unerheblich ist, warum gibt es dann den Parameter? Hmm, ist wohl doch nicht so unerheblich.

    Ich verstehe schon, warum ihr Default-Werte da in Ordnung findet. Ich finde aber, dass so eine enge Kopplung zwischen Deklaration und Aufruf fast immer ungünstig ist. Ausnahme sind sehr allgemeine Funktions-Templates, wo der Standardwert std::less<T>() oder so etwas ist. Das geht, weil kleiner als gängige Konvention zum Vergleichen von Werten ist. Für eine Anzahl von angezeigten Ziffern gibt es aber keinen plausiblen Standardwert.

    Für mich ist das ungefähr so als würde C++ das so spezifizieren:

    float fabs(float = -3.5f);
    

    Wenn man in seinem Programm ganz oft den Absolutwert von -3.5 braucht, ist das super. Man spart damit vier Zeichen pro Aufruf. Für alle anderen ist das unnötig verwirrend, weil sie erstens zunächst begreifen müssen, dass die Funktion eigentlich mehr Parameter hat als Werte übergeben werden. Zweitens müssen sie dann die Deklaration aufsuchen, um den Wert zu erfahren. Drittens wird jeder vernünftige Entwickler den Wert bei einem neuen Aufruf explizit angeben, denn wer weiß, wann irgendjemand den Standardwert mal heimlich ändert. Ist ja bloß ein Standardwert, was kann da schon passieren?



  • TyRoXx schrieb:

    Ausnahme sind sehr allgemeine Funktions-Templates, wo der Standardwert std::less<T>() oder so etwas ist. Das geht, weil kleiner als gängige Konvention zum Vergleichen von Werten ist.

    [...]

    Drittens wird jeder vernünftige Entwickler den Wert bei einem neuen Aufruf explizit angeben, denn wer weiß, wann irgendjemand den Standardwert mal heimlich ändert. Ist ja bloß ein Standardwert, was kann da schon passieren?

    Das widerspricht sich etwas...



  • Ich verstehe den Sinn nicht: Wo ist der Unterschied zwischen einer Funktion mit Defaultargument und zwei Überladungen?
    Und wenn man sie unterschiedlich nennt: Wieso? Spezifiziert man das Argument nicht, macht man schon klar, dass man das Defaultverhalten wünscht.



  • Nathan schrieb:

    Ich verstehe den Sinn nicht: Wo ist der Unterschied zwischen einer Funktion mit Defaultargument und zwei Überladungen?
    Und wenn man sie unterschiedlich nennt: Wieso? Spezifiziert man das Argument nicht, macht man schon klar, dass man das Defaultverhalten wünscht.

    Es sollte kein "Default-Verhalten" geben. Wenn das Verhalten beim Aufruf nicht immer ersichtlich ist, sollte man alle Argumente explizit angeben.

    Dafür gibt es mehrere Gründe:

    - Der Aufruf wird kopiert und woanders eingefügt. Vielleicht ergibt der Standardwert in dem neuen Kontext gar keinen Sinn. Zumindest muss man darüber nachdenken, ob der Wert da Sinn ergibt. Kann ich am Commit erkennen, ob sich da jemand Gedanken gemacht hat? Nö, kann ich nicht.

    - Es kommen immer mehr Parameter dazu (sehr gerne bool s), weil man die ja meistens beim Aufruf nicht sehen kann. Irgendwann fangen Kombinationen von nicht-Default-Werten an, nicht mehr zu funktionieren.

    - In Unit-Test (falls man überhaupt welche hat), werden die Parameter mit Standardwerten natürlich grundsätzlich vergessen.

    - Ich kann den Standardwert nie wieder ändern, falls fremder Code meine Funktion aufruft. Das ist nicht solange nicht schlimm, bis mal jemand versehentlich den Wert ändert.

    - Default-Parameter entstehen oft nur für üble, "temporäre" Hacks und begünstigen diese, weil nicht über die existierenden Aufruf nachgedacht wird.

    - Man kann eigentlich keinen Default-Parameter hinzufügen, ohne sämtliche Aufrufstellen zu überprüfen. Der Wert muss schließlich überall Sinn ergeben. Das ist umständlicher und fehleranfälliger als ein neuer Parameter ohne Standardwert. In der Realität "hofft" man einfach, dass nichts kaputtgeht und überprüft die Aufrufstellen nicht.

    - Wenn der erwartete Wertebereich der Funktion sich ändert, sodass der Standardwert nicht mehr darin liegen würde, müssen auch wieder sämtliche Aufrufstellen überprüft werden.

    - Standardwerte dienen nur der Faulheit. Gebt doch zu, dass ihr einfach nur keine Lust habt, jedes Mal über sinnvolle Werte für zweifelhafte Parameter nachzudenken.

    - Derselbe Werte kann an verschiedenen Stellen aus unterschiedlichen Gründen gewählt worden sein. Ein Standardwert fördert das Weglassen wertvoller Dokumentation der Gründe.

    - Es ist viel zu clever einfach den am häufigsten gewählten Wert zum Standard zu erklären. Das ist so herrlich willkürlich und scheinbar produktiv.

    Das ist alles Kleinvieh, aber das macht bekanntlich auch Mist.



  • @TyRoXx
    Erklär mir bitte was hier böse ist:

    void LogEvent(std::string const& event, boost::optional<std::string> const& data = boost::none);
    


  • Den Grund hatte ich tatsächlich vergessen: Man kann vergessen den zweiten Parameter zu übergeben. Ein explizites boost::none sagt dem Compiler und meinen Kollegen, dass ich da wirklich nichts übergeben wollte.



  • TyRoXx schrieb:

    ...

    Das ist alles Kleinvieh, aber das macht bekanntlich auch Mist.

    Offensichtlich scheint C++ von einem Haufen inkompetenter Deppen entworfen worden zu sein. Da hier reichlich von Defaultwerten gebrauch gemacht wird. Beispiel fstream::open
    Warum ist der Default text und nicht binary?

    Wie kann man nur so eine Sprache nutzen?

    Mfg Martin



  • mgaeckler schrieb:

    Offensichtlich scheint C++ von einem Haufen inkompetenter Deppen entworfen worden zu sein. Da hier reichlich von Defaultwerten gebrauch gemacht wird. Beispiel fstream::open
    Warum ist der Default text und nicht binary?

    Du nennst wirklich die C++-IOStreams ein Beispiel guten Designs?

    Der Standardwert text dürfte weltwelt zu tausenden Bugs in echten Produkten geführt haben und weiterhin führen, weil das binary -Flag vergessen wurde. Ich habe den Fehler bestimmt schon zehn mal gemacht (und hoffentlich immer bemerkt). Hier im Forum kam das Problem auch relativ oft zur Sprache.

    Hier treffen sogar zwei Designfehler zusammen:
    - Ein Standardwert, wo er überhaupt keinen Sinn ergibt.
    - Eine Methode, die zwei völlig unterschiedliche Aufgaben hat, abhängig von einem Standardwertparameter einer ganz anderen Methode.

    Die Bibliothek wurde sicher nicht von Idioten entworfen. Es war aber eine andere Zeit mit anderen Anforderungen. Weder die Rolle, noch die Konventionen von C++ waren damals so gefestigt wie heute.

    (Rust hat das Feature übrigens nicht, falls man Rust noch erwähnen darf.)



  • Ich verstehe deine Argumente.
    Aber es gibt einen Typen von default-Werten, den du vergessen hattest:
    Zusätzliche, optionale Informationen. Bspw.

    int stoi( const std::string& str, std::size_t* pos = 0, int base = 10 );
    

    (Zugegeben, dass int base gehört nicht zu dem Typ, aber ist auch imo total akzeptabel).
    Manchmal braucht man den Index des ersten, nicht konvertierten, Charakters. In den meisten Fällen nicht. Braucht man ihn also, kann man explizit einen Outputparameter übergeben.
    Die tatsächliche Relisierung über Pointer finde ich zugegeben etwas ungeschickt, ein anderer Overload, der einen dummy-Int per Referenz übergibt wäre passender und man spart sich den Branch fürs Prüfen, aber darum gehts nicht.
    Eine anderes Beispiel ist von einer Bibliothek, die ich geschrieben habe:
    Es geht um das Verwalten von Strings. Intern werden die gehasht um schnelle Vergleiche zu ermöglichen und in eine Datenbank eingetragen. Bei der automatischen Generierung von Ids will man evtl., dass sie einzigartig sind. Oder man will verhindern, dass bei einer Kollision der Hashs der Kollisionshandler aufgerufen wird.
    Dann kann man als zusätzlichen Outputparameter im Konstruktor ein enum übergeben, was dann entsprechend gefüllt wird (old_string, new_string, collision).



  • Nathan schrieb:

    int stoi( const std::string& str, std::size_t* pos = 0, int base = 10 );
    

    (Zugegeben, dass int base gehört nicht zu dem Typ, aber ist auch imo total akzeptabel).
    Manchmal braucht man den Index des ersten, nicht konvertierten, Charakters. In den meisten Fällen nicht. Braucht man ihn also, kann man explizit einen Outputparameter übergeben.

    Ich dachte, dass Ausgabeparameter inzwischen endlich gestorben wären.

    Die Signatur ist IMO völlig daneben. Da ist so ziemlich alles falsch. Warum so ein spezifischer Typ wie std::string ? Warum keine Range oder Iteratoren? Warum ein size_t und nicht der korrekte Typ, nämlich in diesem Fall std::string::const_iterator ? Was passiert eigentlich, wenn ich eine negative Basis übergebe?

    template <class InputIterator>
    std::tuple<int, InputIterator> parse_int(InputIterator begin, InputIterator end, unsigned base = 10);
    

    Es ist in der Standardbibliothek üblich, in solchen Fällen mehrere Werte als Tupel zurückzugeben (bisher nur pair ). Was man nicht braucht, kann man ignorieren.

    Als Standardwert ist die 10 hier schon in Ordnung, weil das Konvention unter fast allen Menschen ist und sich zu Lebzeiten von C++ nicht mehr ändern wird.

    Nathan schrieb:

    Eine anderes Beispiel ist von einer Bibliothek, die ich geschrieben habe:
    Es geht um das Verwalten von Strings. Intern werden die gehasht um schnelle Vergleiche zu ermöglichen und in eine Datenbank eingetragen. Bei der automatischen Generierung von Ids will man evtl., dass sie einzigartig sind. Oder man will verhindern, dass bei einer Kollision der Hashs der Kollisionshandler aufgerufen wird.
    Dann kann man als zusätzlichen Outputparameter im Konstruktor ein enum übergeben, was dann entsprechend gefüllt wird (old_string, new_string, collision).

    Verstehe ich nicht.



  • TyRoXx schrieb:

    mgaeckler schrieb:

    Offensichtlich scheint C++ von einem Haufen inkompetenter Deppen entworfen worden zu sein. Da hier reichlich von Defaultwerten gebrauch gemacht wird. Beispiel fstream::open
    Warum ist der Default text und nicht binary?

    Du nennst wirklich die C++-IOStreams ein Beispiel guten Designs?

    Der Standardwert text dürfte weltwelt zu tausenden Bugs in echten Produkten geführt haben und weiterhin führen, weil das binary -Flag vergessen wurde. Ich habe den Fehler bestimmt schon zehn mal gemacht (und hoffentlich immer bemerkt). Hier im Forum kam das Problem auch relativ oft zur Sprache.

    Hier treffen sogar zwei Designfehler zusammen:
    - Ein Standardwert, wo er überhaupt keinen Sinn ergibt.
    - Eine Methode, die zwei völlig unterschiedliche Aufgaben hat, abhängig von einem Standardwertparameter einer ganz anderen Methode.

    Die Bibliothek wurde sicher nicht von Idioten entworfen. Es war aber eine andere Zeit mit anderen Anforderungen. Weder die Rolle, noch die Konventionen von C++ waren damals so gefestigt wie heute.

    (Rust hat das Feature übrigens nicht, falls man Rust noch erwähnen darf.)

    Toll ist es nicht. Es wurde halt das Verhalten der c stdio nachgebildet.

    Im übrigen, Deine Argumente gegen defaults sind ja nicht verkehrt und mein Beispel ist in der Tat ungünstig gewählt, da es unnötig ist, c stdio nach zu bilden. Trotzdem kann es auch gute Gründe dafür (den defaults) geben und man sollte sich dem nicht grundsätzlich verschließen.

    Als der erste c++ standard raus kam, gab es schon min. 8 Jahre brauchbare compiler. Ich weiß noch wie 1990 plötzlich alle meinten c++ lernen zu müssen (ich auch). Es gab also genug Erfahrung.

    Mfg martin



  • mgaeckler schrieb:

    Trotzdem kann es auch gute Gründe dafür (den defaults) geben und man sollte sich dem nicht grundsätzlich verschließen.

    Ich verschließe mich doch gar nicht. Einen guten Grund erkenne ich sehr wohl an: Wenn der Standardwert sogar für projektfremde Programmierer so offensichtlich ist, dass man nicht nachschauen muss. Das ist bei std::sort (fast) der Fall und eine Standardzahlenbasis von zehn ist auch für jeden nachvollziehbar.

    In beiden Fällen könnte man sogar noch darüber streiten, ob der Standardwert jeweils sinnvoll ist. Vergleicht std::sort standardmäßig mit std::less oder mit operator< ? Wer weiß es ohne nachzuschlagen?
    Und ist Basis zehn im EDV-Umfeld wirklich fast immer richtig? Man schreibt Zahlen sehr oft hexadezimal auf, also warum ist der Wert nicht standardmäßig 16?



  • TyRoXx schrieb:

    mgaeckler schrieb:

    Trotzdem kann es auch gute Gründe dafür (den defaults) geben und man sollte sich dem nicht grundsätzlich verschließen.

    Ich verschließe mich doch gar nicht. Einen guten Grund erkenne ich sehr wohl an: Wenn der Standardwert sogar für projektfremde Programmierer so offensichtlich ist, dass man nicht nachschauen muss. Das ist bei std::sort (fast) der Fall und eine Standardzahlenbasis von zehn ist auch für jeden nachvollziehbar.

    In beiden Fällen könnte man sogar noch darüber streiten, ob der Standardwert jeweils sinnvoll ist. Vergleicht std::sort standardmäßig mit std::less oder mit operator< ? Wer weiß es ohne nachzuschlagen?
    Und ist Basis zehn im EDV-Umfeld wirklich fast immer richtig? Man schreibt Zahlen sehr oft hexadezimal auf, also warum ist der Wert nicht standardmäßig 16?

    Dann sind wir uns ja einig, warum also das lange gerede?

    Viele Programme werden für IT-fremde Menschen geschrieben. Viele Konvertierungen finden für diese Menschen statt. Die würden mir den Kopf abreisen, wenn ich denen hexzahlen vorsetzen würde.

    Obwohl: Eine 66jährige, wäre vielleicht froh, wenn ich ihr bescheingen würde, sie sei 42.

    Mfg martin



  • @TyRoXx
    Es ist nicht immer alles was "more verbose" ist auch automatisch "more good".

    Um zu entscheiden was besser ist - verbose oder concise - kommt es mMn. sehr darauf an wie oft man ein bestimmtes Konstrukt in einem Programm findet.
    Wenn man es 3x in einem Programm findet, dann definitiv verbose.
    Wenn man es 3000x findet, dann definitiv concise. Dann muss sich "der neue" halt eine Sache mehr merken bevor er richtig produktiv mittun kann, dafür müssen "die alten" nicht an 3000 Stellen den "noise" mitlesen den sie eh schon kennen.

    Wobei ich mich selbst grundsätzlich eher auf der Seite der "more verbose" Befürworter sehe, aber was du hier predigst geht mir irgendwie zu weit.

    Deine Argumentation, also immer verbose, es sei denn jeder Affe weiss sofort was gemeint ist, muss man nur mehr ein Stückchen weiter treiben, und dann darf man schon überhaupt keine Funktionen mehr verwenden. Die sind böse, die verstecken wertvolle Informationen hintern einem wenigsagenden Namen. Pfui. Weg weg weg.

    TyRoXx schrieb:

    Den Grund hatte ich tatsächlich vergessen: Man kann vergessen den zweiten Parameter zu übergeben. Ein explizites boost::none sagt dem Compiler und meinen Kollegen, dass ich da wirklich nichts übergeben wollte.

    Das ist mMn. kein gutes Argument. Man kann auch mit dem expliziten boost::none vergessen etwas anzugeben. Wenn du 100x sinnfrei boost::none getippt hast, weil deine Events so gut wie nie Daten haben, dann tippst du beim 101-ten Event, der dann doch Daten gehapt hätte, auch boost::none. Bzw. copy-pastest es von einer der anderen 100 Stellen.

    Wenn ich aus irgend einem Grund hier keinen Default-Parameter verwenden dürfte, dann würde ich eher noch zwei Funktionen machen, z.B. LogSimpleEvent(string) und LogDataEvent(string, string) .

    BTW: Was machst du bei Funktionen die z.B. 10+ Parameter nehmen? Guter Stil ist da ja statt dessen ne struct zu übergeben.
    Bei der kannst du dann aber erst nicht erzwingen dass alle Member initialisiert werden - kann man genau so leicht vergessen. Die einzige (mMn. überhaupt nicht gute) Alternative (die mir gerade einfällt), wäre, im Default-Ctor der struct lauter Blödsinnswerte einzutragen und zu beten dass es beim Testen auffällt wenn ein Wert wo nicht explizit gesetzt wurde.



  • TyRoXx schrieb:

    Für mich ist das ungefähr so als würde C++ das so spezifizieren:

    float fabs(float = -3.5f);
    

    Nur das dies ein absolut unsinniges Beispiel ist. Wenn man wie ich oben im Beispiel gennannt den Fall hat das man beim Runden im überwiegenden Fall 2 Nachkommastellen braucht, ist dies keine willkürliche Zahl.

    TyRoXx schrieb:

    Drittens wird jeder vernünftige Entwickler...

    Wie schön hier manche mit Absoluta arbeiten. Merkwürdig nur das ich solchen Entwicklern in meiner gesamten Laufbahn noch nicht in der Praxis begegnet bin. Die Wahrheit liegt wohl wesentlich mehr in einem sinnvollen Mittelweg. Dir ist hoffentlich auch das Paretoprinzip bekannt, das sich auf nahezu alle Bereiche anwenden lässt. In diesem Fall: Wenn in ~80% der Fälle ein Standardwert wirklich Sinn ergibt, ist die Argumentation mit der expliziten Angabe für mich schlicht Unsinn.

    Wenn ich eine Funktion habe die z.B. Werte eines Vectors in eine CSV-getrennte Stringliste umwandelt, und im Programm das Trennzeichen nun einmal üblicherweise "," ist, würde ich eine seperate Funktion für die Sonderfälle, oder eine explizite Angabe in jeden Aufruf ebenso als Unsinn deklarieren.

    Und es gibt sogar Sonderfälle wo das Ändern des Standardwertes auch einen Sinn ergeben kann (das ist aber wirklich eine Ausnahme).


Anmelden zum Antworten