Daten laden und Move



  • Hi,

    ist das so eine gültige (kein UB o.ä.) und sinnvolle art, move zu benutzen (Zeilen 38-43)?

    struct DataObject
    {
    	int classID_;
    	VectorXd data_;
    };
    
    bool load(DataObject& obj, int dim, std::istream& is)
    {
    	is >> obj.classID_;
    	obj.data_.resize(dim);
    	for(int i = 0; i < dim; ++i)
    	{
    		is >> obj.data_(i);
    	}
    
    	return is.good();
    }
    
    struct DataSet
    {
    	int numClasses_;
    	int dim_;
    	std::vector<DataObject> objs_;
    };
    
    DataSet load(const string& fileName)
    {
    	DataSet data;
    
    	ifstream ifs(fileName);
    	if(!ifs)
    	{
    		log() << "failed to open file" << endl;
    		onFatalError(); //stattdessen würde auch exception gehen
    	}
    	ifs >> data.numClasses_ >> data.dim_;
    
    	DataObject obj;	
    	while(load(obj, data.dim_, ifs))
    	{
            ---->   //Hier wird gemoved!! <------
    		data.objs_.push_back(move(obj));
    	}
    
    	log() << "successfully loaded " << fileName << endl;
    	return data;
    }
    

    Sinn ist es, das kopieren in den vektor zu vermeiden.
    Hat nach dem moven die variable auf dem stack noch einen "gültigen" Zustand?
    Im load weise ich danach direkt neue Werte zu, also sollte das eigentlich gehen.

    Außerdem: Findet ihr das so ne gute art, daten zu laden? Ich wollte erst operator>> nehmen, aber leider brauche ich zum laden außerdem die dimension übergeben.

    Und wie würdet ihr die Fehlerbehandlung machen? Im moment ist da das problem, dass die while(load(...)) Schleife sobald etwas falsches in der datei steht einfach abbricht. Das soll auch so sein, weil die datei ja irgendwann zuende ist. Wenn aber vor dem Ende der Datei schon falsche/unerwartete zeichen/daten auftreten, dann sollte das eigentlich anders behandelt werden.



  • Ja, Du darfst in diesem Fall moven.
    Ich würde die Daten binär schreiben.



  • ...und die Daten schlussendlich binär wieder auslesen.



  • Q schrieb:

    ist das so eine gültige (kein UB o.ä.) und sinnvolle art, move zu benutzen (Zeilen 38-43)?

    Der Code ist gültig, aber auf eine ungewollte Weise. Es gibt nämlich keinen Default-Movekonstruktor, das übernimmt der Kopierkonstruktor. Alle Member rüberzumoven geht nämlich nicht, stell dir vor, ein unique_ptr würde das machen (den Pointer rübermoven=rüberkopieren).

    Deshalb musst dir deinen eigenen Movekonstruktor schreiben.

    Aber dann hast du UB.
    Alles, was du mit einem moved-from-Objekt machen darfst, ist ihm es erneut etwas zuweisen oder den Destruktor aufzurufen. resize auf einen gemovten Vektor => UB



  • Q schrieb:

    Außerdem: Findet ihr das so ne gute art, daten zu laden? Ich wollte erst operator>> nehmen, aber leider brauche ich zum laden außerdem die dimension übergeben.

    Das ist so in Ordnung. std::getline ist da ganz ähnlich aufgebaut.

    Ich würde allerdings die Signatur etwas anpassen:

    std::istream& load(std::istream& is, DataObject& obj, std::size_t dim)
    

    (eine alternative Implementation wäre

    is >> obj.classID_;
    obj.data_.reserve(dim);
    std::copy_n(std::istream_iterator<int>(is), dim, std::back_inserter(obj.data_));
    if (!is) is.setstate(std::ios::failbit);
    return is;
    

    )

    Q schrieb:

    Und wie würdet ihr die Fehlerbehandlung machen? Im moment ist da das problem, dass die while(load(...)) Schleife sobald etwas falsches in der datei steht einfach abbricht. Das soll auch so sein, weil die datei ja irgendwann zuende ist. Wenn aber vor dem Ende der Datei schon falsche/unerwartete zeichen/daten auftreten, dann sollte das eigentlich anders behandelt werden.

    Diese Unterscheidung hast du bereits (wenn du Zeile 4 so übernimmst). Bei is.fail() trat ein Fehler auf, bei is.eof() ist alles in Butter.

    PaulaAusmKnast schrieb:

    Ich würde die Daten binär schreiben.

    Besser nicht, damit handelt man sich nur Inkompatibilitäten ein.



  • (etwas von mir gekürzt)

    Q schrieb:

    ist das so eine gültige [...] und sinnvolle art, move zu benutzen [...]?

    struct DataObject
    {
    	int classID_;
    	VectorXd data_;
    };
    
    bool load(DataObject& obj, int dim, std::istream& is)
    // ...
    ;
    
    std::vector<DataObject> load(const string& fileName)
    {
    	std::vector<DataObject> data;
    
    	ifstream ifs(fileName);
    	if(!ifs)
    	{
    		log() << "failed to open file" << endl;
    		onFatalError(); //stattdessen würde auch exception gehen
    	}
    	ifs >> data.numClasses_ >> data.dim_;
    	
    	DataObject obj;	
    	while(load(obj, 42, ifs))
    	{
                    //Hier wird gemoved!!
    		data.push_back(move(obj));
    	}
    
    	log() << "successfully loaded " << fileName << endl;
    	return data;
    }
    

    Ja, das ist in Ordnung so. Allerdings musst du Dich fragen, ob DataObjekt überhaupt einen Move-Konstruktor hat; denn wenn nicht, bringt dir move(...) hier nichts. move(...) ist ja nur eine Möglichkeit, dem Compiler zu sagen, dass man doch lieber den Rvalue-Overload nehmen will. Den gibt es zwar, also push_back(DataObject&&) , aber der kann nicht viel anders machen, als das Objekt doch nur zu kopieren, wenn DataObject keinen Move-Konstruktor hat.

    Laut C++ Standard wird der Compiler in bestimmten Situationen automatische einen Move-Konstruktor erzeugen, der eben elementweise "movet". Ob das hier der Fall ist, hängt glaub'ich davon ab, was VectorXd für ein Typ ist. Außerdem ist diese Automatik noch nicht überall verfügbar. Meines Wissens nach kann der Compiler von Microsoft noch keinen Move-Constructor automatisch erzeugen.

    Q schrieb:

    Hat nach dem moven die variable auf dem stack noch einen "gültigen" Zustand?

    Kommt drauf an. Hat DataObjekt einen Move-Konstruktor? Hat VectorXd einen Move-Konstruktor? Wenn ja, was macht der? Wenn du einen VectorXd, von dem "gemovet" wurde, in load problemlos befüllen kannst, dann sehe ich da kein Problem. Wie gesagt, "Moven" ist keine Compiler-Magie. Da wird auch nur das ausgeführt, was Klassenautoren per Move-Konstruktor definiert haben. In jedem Fall sollte ein "moved-from-Objekt" normal zerstörbar sein; denn der Destruktor von obj wird hier immer am Ende der Funktion ausgeführt, wie es sich für ein Objekt im automatischen Speicher gehört. Und falls ein Objekttyp "Movable" und "Assignable" ist, sollte es auch in einem "moved-from"-Zustand zuweisbar sein. Das ist jetzt kein Muss, aber es ist das, was ich erwarten würde und auch das, was die Standardbibliothek erwartet. Darüberhinaus kannst du natürlich für eigene Typen eigene Garantien machen oder sein lassen.



  • stell dir vor, ein unique_ptr würde das machen (den Pointer rübermoven=rüberkopieren).

    Nicht sicher was du meinst. Machen nicht Move-Ctor/Move-Assignment-Op von unique_ptr genau das?



  • scarymovie schrieb:

    Alles, was du mit einem moved-from-Objekt machen darfst, ist ihm es erneut etwas zuweisen oder den Destruktor aufzurufen.

    Es hält keinen davon ab, eine Klasse zu schreiben, deren Objekte im "moved-from"-Zustand mehr können als nur das.

    scarymovie schrieb:

    resize auf einen gemovten Vektor => UB

    Kannst du das bitte näher begründen? Danke.



  • resize auf einen gemovten Vektor => UB

    Ähm... sofern du std::vector meinst: nein. Der Allokator wird (solange das Allocator-Template-Argument std::allocator ist) kopiert und die Elemente werden geswapped.
    Edit: Naja, so implementiert es zumindest der GCC. 🤡



  • Ich glaube auch nicht, dass scarymovie da Recht hat. Der "moved-from" Zustand von Bibliothekstypen ist laut 17.6.5.15 "valud but unspecified". Und da es für vector<>::resize keine precondition gibt, sehe ich keinen Grund, warum resize zu UB führen sollte. Desweiteren wird verlangt, dass das Moven von StdLib-Containern mit Ausnahme von std::array<> in konstanter Zeit zu passieren hat. Was für ein valider Zustand kann das denn dann noch sein im Falle von vector<> außer der eines leeren Vektors? Ich find's aber schade, dass das explizit nirgens steht (oder ich es wenigstens nicht gefunden habe).

    Die Move-Zuweisung von unique_ptr's

    up = move(up2);
    

    ist zumindest äquivalent zu

    up.reset(up2.release());
    

    definiert worden. Und damit ist klar, in welchen Zustand sich up2 danach befinden sollte.


Log in to reply