Programm stürzt nach korrekter Ausgabe ab.

[wob]

Das behebt jetzt genau 1 Problem, nämlich dass die Namen der Studenten erhalten bleiben, wenn du bei strlen + 1 nicht vergessen hättest (nebst kopieren des Nullbytes). Wozu eine Library-Funktion wie strcpy nutzen, wenn man es auch selbst falsch nachimplementieren kann und wenn es mit std::string 1000x einfacher wäre?

Rule of Zero / Three / Five?

~Student()
    {
        delete this->name;
        this->name = nullptr;
        this->matrikelnummer = 0;
        this->semester = 0;
    }

Das sind absolut nutzlose 3 Zeilen überflüssiger und entbehrlicher Code, der nutzlos ist und weder gebraucht noch verwendet wird

Und außerdem: die zum Kleber-Claus gehörige Gundula heißt "Gause".

[EinNutzer0]

Habe noch einen Default Constructor hinzufügt:

#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];
        for (size_t i = 0; name[i]; i++)
        {
            this->name[i] = name[i];
        }
        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)];
        for (size_t i = 0; s.name[i]; i++)
        {
            this->name[i] = s.name[i];
        }
        this->matrikelnummer = s.matrikelnummer;
        this->semester = s.semester;
    }
    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;
}

Es crasht und ich weiß nicht wieso? Habe auch + 1 hinzugefügt...

[Ein ehemaliger Benutzer]

@EinNutzer0

studarr[i] = stud;

Dadurch wird der Speicher von name am Ende zweimal freigegeben.

[DirkB]

@EinNutzer0 dein eigenes strcpy() kopiert die '\0' am Ende nicht.

[EinNutzer0]

Danke, nu funktioniert s, es ist aber noch nicht richtig, hab das delete entfernt...

#include <cstdlib>
#include <cstring>
#include <iostream>
#include <memory>
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()
    {

        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;
    }
    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;
}

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

Dadurch wird der Speicher von name am Ende zweimal freigegeben

Daran wird es liegen...

[EinNutzer0]

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

Und außerdem: die zum Kleber-Claus gehörige Gundula heißt "Gause"

Ja tut mir leid, schau kein ZDF

[Th69]

Durch das Entfernen des delete hast du ein Memory Leak.
Und es muss delete[] this->name; lauten, denn du allozierst ja ein Array.

Wenn du mal in The rule of three/five/zero schaust, dann wirst du sehen, daß dir noch eine Operation fehlt, welche du implementieren mußt (Hint: Copy-and-swap).

Und da du (dann) sicherlich bemerkt hast, daß das viel zu viel Arbeit für eine relativ einfache Funktionalität ist, solltest du dir unbedingt RAII aneignen, d.h. wenn schon nicht die std:.string benutzt wird, dann wenigstens eine eigene Helperklasse benutzen, welche Ressourcenerzeugung und -löschen durchführt (das dann auch wieder exceptionsicher ist).

[EinNutzer0]

ja... zu komplex, du hast recht... Genug für heute, bis morgen!

[DirkB]

@EinNutzer0 btw: eine Matrikelnummer ist keine Ganzzahl (int), auch wenn sie so aussieht.

Nein, ein unsigned int ist es auch nicht.

Du rechnest schließlich nicht damit. (Ist eine Differenz von Matrikelnummern sinnvoll?)

[EinNutzer0]

@DirkB Das ist ja kein Programm für einen echten Anwendungsfall, vielmehr soll string vermieden werden und später irgendwann auch Vermeidung des vectors.

Quasi ein Entwurf, mit dem @Sedna weiter machen kann...

[SeppJ]

@EinNutzer0 : Du machst ja konsequent genau das, was hier im Thread an der Aufgabenstellung kritisiert wurde. Kein Wunder, dass das nicht läuft und viele Fehler hat, denn die Kritik an der Aufgabenstellung ist schließlich, dass sie zu einem fehleranfälligem Stil führt. Sicher erfüllst du so den Wortlaut der Aufgabenstellung, aber setz dich doch darüber hinweg und mach es "richtig", so dass der Fragesteller sehen kann, dass das Programm dann auf Anhieb korrekt ist und viel schöner anzusehen ist.

[DocShoe]

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.

[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:

1. Die Schrottlösung, die dem Wortlaut der Aufgabe folgt.
2. Eine Lösung, die zwar keine fertigen Komponenten benutzt, aber die Anforderungen der Aufgabenstellung in ein gutes Design verpackt.
3. 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.