Beste Methode für optionale referenz-Parameter



  • Hallo Leute,

    ich habe eine Funktion 'bool isCertValid(X509 *cert)' die in meinem Fall Zertifikate bzw. Zertifikatketten überprüft und bei Erfolg 'true' zurückgibt. Im Falle eines ungültigen Zertifikats, würde ich aber gerne die Details erfahren, die dazu führten. Nun ist die Überlegung in die Funktion einen zweiten Parameter einzubauen, in der die Fehlerdetails zurückgegeben werden. Kurz gesagt: ich möchte einen Referenzparameter nutzen, der optional ist. Welche Methode wäre am besten?

    Folgende Möglichkeiten fallen mir da ein:

    Methode 1: die Funktion überladen

    void myFkt1(int val, std::string &optionalErrMsg)
    {
    	optionalErrMsg = "myFkt1 error details";
    }
    
    void myFkt1(int val)
    {
    	std::string errMsg; // wird nicht weiter genutzt
    	myFkt1(val, errMsg);
    }
    
    int main()
    {
    	std::string errMsg1;
    
    	myFkt1(5);
    	myFkt1(5, errMsg1);
    
    	std::cout << errMsg1 << std::endl;
    
    	return 0;
    }
    

    Ich weiß nicht warum, aber ich mag diese Methode nicht, wenn es darum geht nur einen optionalen Parameter hinzuzufügen.

    Methode 2: eine dummy-Variable verwenden

    static std::string __dummy__;
    void myFkt2(int val, std::string &optionalErrMsg = __dummy__)
    {
    	optionalErrMsg = "myFkt2 error details";
    }
    
    int main()
    {
    	std::string errMsg2;
    
    	myFkt2(5);
    	myFkt2(5, errMsg2);
    
    	std::cout << errMsg2 << std::endl;
    
    	return 0;
    }
    

    Irgendwie habe ich das Gefühl, dass es keine elegante Lösung ist.

    Methode 3: Pointer auf Pointer

    void myFkt3(int val, std::string **optionalErrMsg = nullptr)
    {
    	if (optionalErrMsg != nullptr)
    	{
    		std::string *newMsg = new std::string();
    		*newMsg = "myFkt3 error details";
    		*optionalErrMsg = newMsg;
    	}
    }
    
    int main()
    {
    	std::string *errMsg3 = nullptr;
    
    	myFkt3(5);
    	myFkt3(5, &errMsg3);
    
    	std::cout << *errMsg3 << std::endl;
    
    	if (errMsg3 != nullptr)
    		delete errMsg3;
    
    	return 0;
    }
    

    Widerlich! Man muss zudem noch die Ressourcen freigeben. Allerdings wird diese Methode sehr oft in OpenSSL verwendet.

    Von den drei Varianten tendiere ich noch eher zu der ersten. Gibt es etwas besseres?

    viele Grüße,
    SBond



  • Definitiv überladen.

    Und wenn man sich den Overhead ein Ergebnis zu erzeugen das dann verworfen wird sparen will, kann man es immer noch so implementieren

    void myFkt1Impl(int val, std::string *optionalErrMsg)
    {
        if (optionalErrMsg)
            *optionalErrMsg = "myFkt1 error details";
    }
    
    void myFkt1(int val, std::string &optionalErrMsg)
    {
        myFkt1Impl(val, &optionalErrMsg);
    }
    
    void myFkt1(int val)
    {
        myFkt1Impl(val, 0);
    }
    

    Alternativ kann man auch gleich die myFkt1Impl Variante public machen.



  • vielen Dank 😃



  • Ich würde das so machen:

    void myFkt1(int val, boost::optional<std::string &> optionalErrMsg);
    

    Ich halte es für fragwürdig Fehlermeldungen implizit verschwinden zu lassen, wenn gerade jemand aus Faulheit oder Dummheit vergisst einen Puffer dafür zu übergeben. Deswegen auf keinen Fall Überladung oder ein Default.

    Was spricht überhaupt gegen die einfache Variante?

    std::string myFkt1(int val);
    


  • boost::optional<std::string &> ? Ernsthaft?
    Wieso nicht einfach std::string* ?

    Der weniger interessante/wichtige Rest:

    TyRoXx schrieb:

    Ich halte es für fragwürdig Fehlermeldungen implizit verschwinden zu lassen, wenn gerade jemand aus Faulheit oder Dummheit vergisst einen Puffer dafür zu übergeben.

    Ja das mit der Faulheit...
    Stimmt schon, man sollte es Leuten nicht all zu leicht machen schlechten Code zu schreiben. Andrerseits, wenn jemand drauf scheissen will, dann tut er das auch. Dann schreibt er seinen std::string dummy; und übergibt dem beim Aufruf. Und ignoriert ihn dann trotzdem im Fehlerfall.

    Im Endeffekt geht es mMn. eher darum wie häufig man Fälle einschätzt wo die Fehlermeldung korrekterweise ignoriert wird. Wenn man damit rechnet dass das oft sein wird, dann finde ich es irgendwie schon angebracht einen Overload anzubieten.

    Wobei solche Fälle nach meiner Erfahrung meist so aussehen, dass man in nur ganz bestimmte Fehler ignorieren möchte. Nämlich die, die eben häufig auftreten können und sozusagen "erwartet" sind.
    Beispielsweise wenn ich sage "versuch mir mal ein File zu laden", und das File ist gar nicht da, dann will man das eben oft ignorieren. Aber nur das. Andere Fehler, wie IO Fehler beim Laden des Inhalts, Parsing-Fehler wenn der Inhalt geparsed wird etc. will man aber nicht ignorieren.

    In dem Fall würde ich vorschlagen die API komplett zu ändern. Man macht eine LoadFile() Funktion und eine TryLoadFile() Funktion. Die LoadFile() Funktion wirft immer eine Exception wenn irgendwas daneben gegangen ist. Und die TryLoadFile() Funktion gibt für "file not found" einfach false zurück, und wirft für alle anderen Fehler eine Exception. (Bzw. wenn es 2-3 verschiedene Dinge sind die man ignorieren will, dann kann man nen enum zurückgeben. Etwas in der Art.)
    Natürlich hat auch dieses Pattern Grenzen. Manchmal gibt es Fälle wo verschiedene Aufrufer ganz verschiedene Fehler ignorieren möchten bzw. nicht ignorieren möchten. Dort wo es anwendbar ist, ist es mMn. aber recht praktisch - auf jeden Fall etwas was man in seinem Repertoire haben sollte.

    Und das "aus Dummheit" Argument... naja, weiss nicht. IDEs haben typischerweise etwas wie Intellisense, und wenn ich " x.Blub( " getippt habe, und da dann " X::Blub(...) (#1 of 2 overloads) " in der Bubble steht, dann ... muss ich schon sehr dumm sein um nicht zu checken dass der 2. Overload vielleicht was interessantes anbieten könnte. Und wenn ich nicht nachgucke was es ist, dann sind wir wieder bei der Faulheit 🙂



  • TyRoXx schrieb:

    Ich würde das so machen:

    void myFkt1(int val, boost::optional<std::string &> optionalErrMsg);
    

    Oder als Rückgabewert. Dann bräuchte man die Referenz nicht.



  • die Verwirrung ist ein wenig mir geschuldet!
    Ich habe das Beispiel mit void-Funktionen() erstellt. Wahrscheinlich um es einfacher zu halten und so ist es mir nicht mehr direkt aufgefallen.

    SBond schrieb:

    ich habe eine Funktion 'bool isCertValid(X509 *cert)' [...]

    Das ist etwas untergegangen. Es geht also um Funktionen, die true/false zurückgeben. In meinem Falle konnte ein 'false' so viele Ursachen haben, dass ich optional den genauen Grund zurückgeben lassen kann.

    ..wollte es zur Vollständigkeit noch mal erwähnen. Habe es jetzt mit der vorgeschlagenen Überladung der Funktion gelöst.
    nochmals Danke für die Tipps 🙂



  • hustbaer schrieb:

    boost::optional<std::string &> ? Ernsthaft?
    Wieso nicht einfach std::string* ?

    "Wieso nicht noch einfacher char** ?"

    Rohe Zeiger sind schlecht, weil man jedes Mal darüber nachdenken muss, was gemeint ist.
    - Darf da nullptr übergeben werden?
    - Ist das ein besitzender Zeiger?
    - Ist das ein Iterator?



  • TyRoXx schrieb:

    hustbaer schrieb:

    boost::optional<std::string &> ? Ernsthaft?
    Wieso nicht einfach std::string* ?

    "Wieso nicht noch einfacher char** ?"

    Total unpassender Vergleich.

    TyRoXx schrieb:

    Rohe Zeiger sind schlecht, weil man jedes Mal darüber nachdenken muss, was gemeint ist.
    - Darf da nullptr übergeben werden?
    - Ist das ein besitzender Zeiger?
    - Ist das ein Iterator?

    Ich verstehe grundsätzlich was du meinst. Ich halte es aber bei diesem Beispiel für etwas extrem.
    Anhand des Funktionsnamens und Parameternamens ist die Semantik klar.
    Anhand der Semantik ist es klar dass es nicht um ein Array geht.
    Da es nicht um ein Array geht, und es keine Referenz ist, ist klar dass man nullptr übergeben darf - sonst hätte man ja ne Referenz genommen.
    Anhand der Semantik ist auch klar dass keine Ownership übernommen wird.
    Und was du mit Iterator meinst verstehe ich nicht.

    Wie gesagt, ich verstehe was du meinst, und will nicht grundsätzlich widersprechen, weil ich das grundsätzlich ähnlich sehe wie du. Also dass so viel wie möglich dadurch kommuniziert werden sollte z.B. welche Typen man verwendet. Hab' mich ja neulich erst mit Shade deswegen gezankt.

    Bei nem optionalen Output-Parameter halte ich es trotzdem für übertrieben. Und dass optional<T&> besser sein soll, .... ne. optional<T&> ist abschäulich.



  • hustbaer schrieb:

    Bei nem optionalen Output-Parameter halte ich es trotzdem für übertrieben. Und dass optional<T&> besser sein soll, .... ne. optional<T&> ist abschäulich.

    Wenn der korrekte Typ abscheulich ist, dann bevorzuge ich abscheuliche Signaturen. optional drückt exakt das aus, was ausgedrückt werden soll an der Stelle. Ich sehe keinen Grund den weniger präzisen Zeiger zu benutzen.

    Wo hört optional denn auf abscheulich zu sein?
    optional<uint32_t> vs int64_t mit -1 für none ?
    optional<T *> vs T ** ?

    Ich finde es übrigens faszinierend, dass es bei so einfachem Kram noch gegensätzliche Meinungen gibt. Vielleicht müsste ich meine Mini-Design-Entscheidungen mal im Detail begründen, in einem Blog oder so. Vielleicht bekomme ich dann andere Gegenargumente zu sehen als die ewige Macht der Gewohnheit.

    hustbaer schrieb:

    Anhand des Funktionsnamens und Parameternamens ist die Semantik klar.
    Anhand der Semantik ist es klar dass es nicht um ein Array geht.
    Da es nicht um ein Array geht, und es keine Referenz ist, ist klar dass man nullptr übergeben darf - sonst hätte man ja ne Referenz genommen.
    Anhand der Semantik ist auch klar dass keine Ownership übernommen wird.
    Und was du mit Iterator meinst verstehe ich nicht.

    Das ist eben nicht alles klar. Es erfordert Denkaufwand und ich bin denkfaul. Ich nutze Typinformationen, um sehr schnell einen Programmausschnitt verstehen zu können. Je eindeutiger die Typen sind, desto schneller geht das. Ich merke bei der Arbeit mit anderen Entwicklern, dass ich Code viel schneller begreife als die meisten anderen. Jede Unschönheit fällt mir dabei auf. Der Durchschnittsentwickler wird dadurch absolut gesehen auch verlangsamt, aber das fällt kaum auf, weil er sowieso langsam ist. </theorie>



  • Nene, nicht der Text optional<Blub> ist abschäulich.
    Auch nicht der Text optional<Blub&> .
    Sondern dass die Semantik von optional<Blub&> alles andere als klar ist.

    optional<Blub&> oref = ...;
    Blub b1;
    oref = b1;   // Verbiegt die Referenz auf b1 - etwas was mit "normalen" Referenzen nicht geht
    *oref = ...; // Ändert den Wert von b1
    

    *schauder*

    Wenn es darum geht Dinge zu schreiben die intuitiv und einfach und klar sind, dann finde ich das nicht gut.



  • hustbaer schrieb:

    optional<Blub&> oref = ...;
    Blub b1;
    oref = b1;   // Verbiegt die Referenz auf b1 - etwas was mit "normalen" Referenzen nicht geht
    *oref = ...; // Ändert den Wert von b1
    

    Ja, so eine implizite Konvertierung von T& zu optional<T&> ist ungünstig. Am besten möglichst viele lokale Variablen const machen, gerade optional s. Wenn das optional nicht const sein kann, weil die Funktion zu komplex ist, wäre es mit einem Zeiger auch nicht mehr einfach und klar.

    Ich habe schon darüber nachgedacht ein Interface mit weniger impliziten Konvertierungen zu verwenden. Bisher brauchte ich das aber nicht unbedingt. So etwas wie optional<T&> verwende ich äußerst selten.

    optional<Blub&> oref = ...;
    Blub b1;
    oref = some(b1);   // Verbiegt die Referenz auf b1 - etwas was mit "normalen" Referenzen nicht geht
    *oref = ...; // Ändert den Wert von b1
    

    Eine Lösung habe ich noch:

    template <class BackInsertionRange>
    void myFkt1(int val, BackInsertionRange &&optionalErrMsg)
    {
    	insert_at_end(optionalErrMsg, string_view("error!!!!"));
    }
    
    struct null_buffer
    {
    	std::nullptr_t end() const
    	{
    		return nullptr;
    	}
    
    	template <class Iterator>
    	void insert(std::nullptr_t, Iterator, Iterator)
    	{
    	}
    };
    
    int main()
    {
    	myFkt1(1, null_buffer());
    	std::string error;
    	myFkt1(1, error);
    }
    

    Sobald wir Concepts haben, halte ich dieses Design für sinnvoller als irgendwelche Nullable-Typen.



  • Das ist doch gröbstes Overdesign. Das ist dir schon klar, oder?



  • hustbaer schrieb:

    Das ist doch gröbstes Overdesign. Das ist dir schon klar, oder?

    Nein, warum das denn?



  • Wie wärs mit

    #include <iostream>
    #include <string>
    
    struct EmptyErrorCallback { void operator()(std::string const &){} };
    
    template <typename ErrorCallback = EmptyErrorCallback>
    bool do_something(int x, ErrorCallback error_callback = EmptyErrorCallback())
    {
    	if (x == 0) 
    	{ 
    		error_callback("Zero is not allowed!"); 
    		return false;
    	}
    	return true;
    }
    
    int main()
    {
    	auto error_handler = [](std::string const &msg)
    	{
    		std::cerr << msg << "\n";
    	};
    
    	do_something(1); // No error
    	do_something(0); // Error, do nothing
    	do_something(0, error_handler); // Error, show message
    }
    

    ?

    Keine Ahnung ob das gut ist, aber ich mag callbacks 😃



  • Für das besagte Problem überdimensional, solch eine Implementierung mit Callbacks ist in einigen Fällen jedoch recht nützlich.



  • Zeiger, nullptr + Dokumentation über der Funktion.
    Dann weiß jeder was sache ist, kennt das von C Apis und man muss kein tam tam auffahren.

    (gegebenenfalls exceptions verwenden)


Log in to reply