3. Programm stürzt nach korrekter Ausgabe ab.

Programm stürzt nach korrekter Ausgabe ab.

  • E
    Gesperrt

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

    dem TE einfach die Hausaufgaben zu machen, aber eben mit einem selbstgeschriebenem Vector als unterliegender Datenstruktur

    Das seh ich auch so... Hier wäre mein Vorschlag mit char * anstatt string und ohne vector:

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

class Student
{
public:
    Student(const char *name, int matrikelnummer, int semester)
    {
        this->name = new char[strlen(name)];
        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()
{
    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;
}

int main()
{
    while (true)
    {
        printKlausAndGundula();
    }
}
    0
    D
     1 Antwort

  • @EinNutzer0
    Hast du den Code mal laufen lassen? Das sollte wunderschön wegcrashen so wie du es hier gepostet hast.

    EDIT: Ne, sorry, du musst noch ein bisschen was ändern damit es crasht.
    EDIT2: strlen Bug übersehen, d.h. ja doch, hat auch so gutes Crash-Potential.

    0
  • W

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

    0
    E
     1 Antwort
  • E
    Gesperrt

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

    0
    ?
    D
     2 Antworten
  • ?

    @EinNutzer0

    studarr[i] = stud;

    Dadurch wird der Speicher von name am Ende zweimal freigegeben.

    0
  • D

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

    0
  • E
    Gesperrt

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

    0
  • E
    Gesperrt

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

    0

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

    1
  • E
    Gesperrt

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

    0
  • D

    @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?)

    0
  • E
    Gesperrt

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

    0
  • Mod

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

    0 1 Antwort

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

    0 1 Antwort
  • Mod

    @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.
    0
  • E
    Gesperrt

    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;
    }
}
    0
  • W

    Deine Implementierung des operator= ist fehlerhaft.

    0
    E
     1 Antwort
  • E
    Gesperrt

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

    Deine Implementierung des operator= ist fehlerhaft.

    Wie geht das richtig?

    0 1 Antwort

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

    Wie geht das richtig?

    Google: Copy And Swap Idiom

    0
    W
     1 Antwort
  • W

    @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

    0
Anmelden zum Antworten