Programm stürzt nach korrekter Ausgabe ab.

SeppJ

@DocShoe sagte in Programm stürzt nach korrekter Ausgabe ab.:

Oder gib zwei Lösungen ab. Einmal die, die der Dozent erwartet und zeigt, wie man´s nicht macht und die, die std::string benutzt und es richtig macht.

Besser noch 3:

Die Schrottlösung, die dem Wortlaut der Aufgabe folgt.
Eine Lösung, die zwar keine fertigen Komponenten benutzt, aber die Anforderungen der Aufgabenstellung in ein gutes Design verpackt.
Den Fünfzeiler, der mit den fertigen Komponenten der Standardbibliothek die (funktionalen) Anforderungen der Aufgabenstellung erfüllt.

EinNutzer0

Scheiße, das läuft jetzt so lange, bis mein kompletter Arbeitsspeicher (16GB) voll ist:

#include <cstdlib>
#include <cstring>
#include <iostream>
using namespace std;

class Student
{
public:
    Student() : Student("default constructor", 1, 1)
    {
    }
    Student(const char *name, int matrikelnummer, int semester)
    {
        this->name = new char[strlen(name) + 1];
        strcpy(this->name, name);
        this->matrikelnummer = matrikelnummer;
        this->semester = semester;
    }
    ~Student()
    {
        delete[] this->name;
        this->name = nullptr;
        this->matrikelnummer = 0;
        this->semester = 0;
    }
    Student(const Student &s)
    {
        this->name = new char[strlen(s.name) + 1];
        strcpy(this->name, s.name);
        this->matrikelnummer = s.matrikelnummer;
        this->semester = s.semester;
    }
    Student &operator=(const Student &s)
    {
        this->name = new char[strlen(s.name) + 1];
        strcpy(this->name, s.name);
        this->matrikelnummer = s.matrikelnummer;
        this->semester = s.semester;
        return *this;
    }
    void setSemester(int semester)
    {
        this->semester = semester;
    }
    friend ostream &operator<<(ostream &os, const Student &s);

private:
    char *name;
    int matrikelnummer;
    int semester;
};
ostream &operator<<(ostream &os, const Student &s)
{
    os << s.name << " ; " << s.matrikelnummer << " ; " << s.semester;
    return os;
}

void printKlausAndGundula()
{
    const int n = 5000;
    Student stud = Student("Kleber, Klaus", 1234, 1);
    Student stud2 = Student("Gauss, Gundula", 5678, 1);
    Student stud3 = stud2;
    stud2.setSemester(3);
    cout << stud << endl;
    cout << stud2 << endl;
    cout << stud3 << endl;
    Student studarr[n];
    for (size_t i = 0; i < n; i++)
    {
        int r = rand() / (RAND_MAX / 3);
        switch (r)
        {
        case 0:
            studarr[i] = stud;
            break;
        case 1:
            studarr[i] = stud2;
            break;
        case 2:
            studarr[i] = stud3;
            break;
        default:
            break;
        }
    }
    cout << studarr[0] << endl;
    cout << studarr[1] << endl; // Aborted (Speicherabzug geschrieben)
    cout << studarr[n - 2] << endl;
    cout << studarr[n - 1] << endl;
}

int main()
{
    while (true)
    {
        printKlausAndGundula();
        cout << endl;
    }
}

wob

Deine Implementierung des operator= ist fehlerhaft.

EinNutzer0

@wob sagte in Programm stürzt nach korrekter Ausgabe ab.:

Deine Implementierung des operator= ist fehlerhaft.

Wie geht das richtig?

hustbaer

@EinNutzer0 sagte in Programm stürzt nach korrekter Ausgabe ab.:

Wie geht das richtig?

Google: Copy And Swap Idiom

wob

@hustbaer sagte in Programm stürzt nach korrekter Ausgabe ab.:

Google: Copy And Swap Idiom

Übrigens wurde dieses Idiom in diesem Thread weiter oben von @Th69 schon erwähnt. Hinweise, die hier gegeben wurden, dürfen auch gerne beachtet werden

Du (also @EinNutzer0) darfst dir ansonsten auch überlegen, warum deine Implementierung falsch ist. Hint: was seht in der Variablen self->name, bevor du mit this->name = new char[strlen(s.name) + 1]; einen neuen Wert zuweist?

Siehe außerdem: http://gotw.ca/gotw/059.htm

Schlangenmensch

@EinNutzer0 Das ist ein typisches Beispiel dafür, warum manuelles Speichermanagement nicht so einfach ist, wie es vieleicht manchmal scheint.

Du erstellst ein Objekt von Student, das Objekt reserviert Speicher für den String. Dann wird dem Objekt ein anderes Objekt zugewiesen. Das Der char* zeigt auf den bereits reservierten Speicher. Du reservierst neuen Speicher und lässt den Pointer auf den neu reservierten Speicher zeigen. Und der vorher bereits reservierte Speicher fliegt irgendwo rum und du kommst nicht mehr dran.

EinNutzer0

Ok, ich hab hier mal geschaut: https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom :

    Student &operator=(Student other) // (1)
    {
        swap(*this, other); // (2)

        return *this;
    }
    friend void swap(Student &first, Student &second) // nothrow
    {
        // enable ADL (not necessary in our case, but good practice)
        using std::swap;

        // by swapping the members of two objects,
        // the two objects are effectively swapped
        swap(first.name, second.name);
        swap(first.matrikelnummer, second.matrikelnummer);
        swap(first.semester, second.semester);
    }

Jetzt brauch die .exe bescheidene 1,1 MB Arbeitsspeicher und es wird kein Speicherabzug geschrieben. Ich muss nur noch verstehen, wieso. Und seh ich es richtig, dass beim = jetzt iwie "halbflach" kopiert wird?

Anmerkung: @ TE : Ich kann mir nicht so richtig vorstellen, dass ihr das tatsächlich "so" machen sollt...

@hustbaer et al. Danke für den Suchbegriff.

wob

@EinNutzer0 sagte in Programm stürzt nach korrekter Ausgabe ab.:

Und seh ich es richtig, dass beim = jetzt iwie "halbflach" kopiert wird?

Nein. Es wird eine deepcopy gemacht (also normalweise implementiert man es so). Du erzeugst dir die Kopie bereits in der mit (1) markierten Zeile. Da steht operator=(Student other), nicht operator=(Student &other). Wichtiger Unterschied.

In dem gotw-Link (etwas veraltet, da von vor C++11) wird die Kopie dagegen im Body des operator= erzeugt - in der Zeile mit T temp( other );. Das ist natürlich auch ok.

Bringt mich noch gleich dazu, dass du in C++11 und neuer auch noch den Move-Assignment-Operator und den Move-Konstuktor schreiben könntest/solltest (Rule of 5)... Eben verdammt viel Arbeit.

Schlangenmensch

Wobei Copy and Swap den Move Kram direkt mit liefert

Student (Student&& other):
Student()
{
    swap(*this, other); // (2)
}

und fertig

du könntest ja theoretisch auch vor dem anfordern des speichers überprüfen, ob überhaupt schonmal speicher angefordert wurde (nullptr) und diesen speicher dann evtl. freigeben.

wob

@Schlangenmensch sagte in Programm stürzt nach korrekter Ausgabe ab.:

Wobei Copy and Swap den Move Kram direkt mit liefert

???

Bitte genauer erklären. Die Idee ist doch, dass ich mir sowas wie "new" sparen kann, da ich den Pointer aus other klaue. Ich will also gerade nicht erst einen Standard-Studenten mit Standard-Name erstellen.

EinNutzer0

Hier nochmal genauer, was passieren sollte... (und auch schon passiert) (habe mein feinstes Englisch ausgepackt...)

void printKlausAndGundula()
{
    const int n = 5000;
    Student stud = Student("Kleber, Claus", 1234, 1);
    Student stud2 = Student("Gausel, Gundula", 5678, 1);
    Student stud3 = stud2;
    stud2.setSemester(3);
    cout << stud << endl;  // should print 1, because of deep copy
    cout << stud2 << endl; // should print 3, because of deep copy
    cout << stud3 << endl; // should print 1, because of deep copy
    Student studarr[n];
    for (size_t i = 0; i < n; i++)
    {
        int r = rand() / (RAND_MAX / 3);
        switch (r)
        {
        case 0:
            studarr[i] = stud;
            break;
        case 1:
            studarr[i] = stud2;
            break;
        case 2:
            studarr[i] = stud3;
            break;
        default:
            break;
        }
    }
    stud.setSemester(0);
    stud2.setSemester(0);
    stud3.setSemester(0);
    cout << studarr[0] << endl;       // should print either 1 or 3, because of deep copy through assignment
    cout << studarr[1] << endl;       // should print either 1 or 3, because of deep copy through assignment
    cout << studarr[n - 2] << endl;   // should print either 1 or 3, because of deep copy through assignment
    cout << studarr[n - 1] << endl;   // should print either 1 or 3, because of deep copy through assignment
}

Schlangenmensch

@wob In dem Beispiel ist der Standard Konstruktor halt blöd, weil der aus einem mir unbekannten Grund bereits Speicher für einen Nichtssagenden String anlegt. Wenn man sich das spart kann man beim Move Konstruktor einfach ein leeres Objekt erstellen und other da rein 'swappen'.
Für move assignment muss man nichts machen, da der bereits definierte mit "call by value" dann, wenn er mit einem r-value aufgerufen wird, den move Konstruktor aufruft.

EinNutzer0

Den Standard Konstruktor habe ich aufgrund dieser Zeile gebraucht:

@EinNutzer0 sagte in Programm stürzt nach korrekter Ausgabe ab.:

Student studarr[n];

also für das n-malige Initialisieren der (auto) Array Elemente. Wenn ihr mir sagt, wie es anders ginge... dann immer her damit.

Student *studentmemptr;

also wenn schon dynamische speicherverwaltung, dann richtig.

Schlangenmensch

@EinNutzer0 du musst aber im Standard Konstruktor nicht unbedingt mit new schon neuen Speicher anfordern. Wenn du den Standard Konstruktor von std::string oder vector aufrufst, schreibt der ja auch nicht per default irgendwas da rein.

Swordfish

@Wade1234 sagte in Programm stürzt nach korrekter Ausgabe ab.:

du könntest ja theoretisch auch vor dem anfordern des speichers überprüfen, ob überhaupt schonmal speicher angefordert wurde (nullptr) und diesen speicher dann evtl. freigeben.

Nix prüfen. delete nullptr ist wohldefiniert. Aber Du hast dann den selben Käse wenn das nächste new wirft.

die ausnahme kann man ja abfangen, oder?

Swordfish

@Wade1234 Schon, aber die alten Daten sind weg. Ich zumindest bin (hoffentlich nicht alleine) der Meinung daß assignment operators das Objekt in seinem ursprünglichen Zustand belassen sollen wenn sie ihre Aufgabe nicht erfüllen können.