About a code. Code Style Frage.

Skym0sh0

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

Nathan

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.

TyRoXx

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.

hustbaer

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

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

TyRoXx

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.

mgaeckler

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

TyRoXx

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

Nathan

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

TyRoXx

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.

mgaeckler

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

TyRoXx

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?

mgaeckler

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

hustbaer

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

asc

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

TyRoXx

hustbaer schrieb:

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.

Du argumentierst jetzt aber mit einem Fehler, der bei beiden Varianten passieren kann. Der Standardwert ist eine davon unabhängige, weitere Fehlerquelle.

hustbaer schrieb:

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

Das ist noch besser als mit optional zu arbeiten. Mit dem optional<string> const& müsste jeder lvalue- string kopiert werden. optional ist hier also die falsche Wahl, unabhängig von Standardparameterwerten.
Explizit unterschiedlich benannte Funktionen halte ich im Zweifel für die bessere Wahl, weil das die Intention deutlicher macht. Ich sehe, dass die zwei Log-Funktionen wohl unterschiedliche Dinge tun. Das muss ich dann nicht an der Anzahl der Argumente erraten.

hustbaer schrieb:

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.

Man kann prinzipiell nicht verhindern, dass jemand Unsinnswerte übergibt. Man kann nur mit einem Konstruktur dabei helfen alle Member zu setzen. Ich sehe nicht, was das mit Standardparametern zu tun hat.

hustbaer

TyRoXx schrieb:

hustbaer schrieb:

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.

Du argumentierst jetzt aber mit einem Fehler, der bei beiden Varianten passieren kann. Der Standardwert ist eine davon unabhängige, weitere Fehlerquelle.

Wo? Wie?
Entweder ich gebe den Wert an, oder ich gebe ihn nicht an. boost::none ist ein vernünftiger Default-Wert in dem skizzierten Fall, an dem sich wohl so schnell nix ändern wird (genauer: niemals was ändern wird^*). Wo soll da eine weitere, zusätzliche Fehlerquelle sein? WTF?

TyRoXx schrieb:

hustbaer schrieb:

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

Das ist noch besser als mit optional zu arbeiten. Mit dem optional<string> const& müsste jeder lvalue- string kopiert werden. optional ist hier also die falsche Wahl, unabhängig von Standardparameterwerten.

Herr TyRoXx. Das Wort Verhältnismässigkeit ist dir vermutlich fremd. Schlag es nach, ist ne interessante Sache. Ansonsten denk dir halt einfach nen optional<int> damit der Ineffizienzalarm in deinem Kopf nicht so schlimm losbrüllt.

TyRoXx schrieb:

hustbaer schrieb:

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.

Man kann prinzipiell nicht verhindern, dass jemand Unsinnswerte übergibt. Man kann nur mit einem Konstruktur dabei helfen alle Member zu setzen. Ich sehe nicht, was das mit Standardparametern zu tun hat.

Boah, stell dich mal nicht so blöd!

LogEvent("foo");
//...
LogEvent("foo", "foo-data");

// vs.

EventEntry ee;
ee.name = "foo";
// Weitere 9 Werte setzen die man immer setzen muss
LogEvent(ee);
// ...
EventEntry ee;
ee.name = "foo";
// Weitere 9 Werte setzen die man immer setzen muss
ee.data = "foo-data";
LogEvent(ee);

Du erkennst hier kein Muster? Echt nicht?

^*: Natürlich könnte man auf die Idee kommen boost::none durch boost::optional<T>() zu ersetzen, was ne Dependency auf none.hpp einspart. Ändert aber an der Semantik nixe.

mgaeckler

Mal ein Beispiel:

Ich habe eine Funktion, die liefert alle Knoten eines xml Baums mit einem bestimmten tag. Fast alle Programme lesen und schreiben ihre xml Dateien selbst und kümmern sich nicht um namespaces und prefixe. Ein paar aber doch. Diese dürfen den Namen eines namespaces mit angeben, der per default leer ist. Ohne default Parameter müsste ich von einigen funktionen zwei Versionen erstellen, die sich nur darin unterscheiden, den zweiten Parameter weiter zu geben. Irgendwo wird halt dann aus dem namespacenamen ein prefix ermittelt und mit dem xml prefix verglichen.

Mfg Martin

TyRoXx

hustbaer schrieb:

...

Versuch doch mal nachzuvollziehen was ich schreibe und warum. Ich mache das nicht, um jemanden zu ärgern. Dass dir etwas nicht passt, heißt nicht automatisch, dass es falsch ist.

Dein Ton passt nicht zur bisher sachlichen Diskussion. Es wäre schön, wenn du dich mäßigen und auf eine professionelle Ebene zurückkommen würdest.

Jockelx

Ich muss zZ java machen und hab oft:

String s;
...
s.toLowerCase(Locale.getDefault());

Ich finde es bescheuert, da immer getDefault() aufrufen zu müssen, um einen Default-Parameter zu setzen.
Hier zählt auch das Argument 'jemand könnte den Default Wert mit schlimmen Konsquenzen ändern' nicht, da getDefault() auch geändert werden könnte.

TyRoXx

Wenn du das wirklich oft schreiben musst, machst du etwas falsch. Du solltest eine Methode definieren, die diese Umwandlung durchführt und semantisch sinnvoll benannt ist.

String normalizeExampleString(String denormalized)
{
    //we use the default locale here because [...]
    return String.toLowerCase(Locale.getDefault());
}