Programm stürzt nach korrekter Ausgabe ab.

class Verwaltung:Student

Eine Verwaltung ist ein Student?

wob

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

"Implementieren Sie eine Klasse Student, die einen Namen, Vornamen (beides als char*),

Bäh. Warum nur? Ok, man sollte wissen, wie char* funktionieren, aber man würde hier einfach std::string nehmen (oder etwas entsprechendes).

Implementieren Sie eine weitere Klasse Verwaltung, die ein Array von Studenten enthält. Diese Klasse soll neue Studenten anlegen

Nennt sich std::vector<Student>. Gut, sollst du hier selbst machen.

Student(const char name[],const char vorname[], unsigned fachsem){
  (...)
        this->matnr = matnr+setMatnr();

Hier habe ich gesetutzt. Diese Zeile ist verwirrend. Und zwar liegt das daran, dass erstens this->matnr und matnr dieselbe Variable sind (warum einmal mit, einmal ohne this?), zweitens überraschenderweise matnr bereits unten auf 1111 gesetzt wird und wenn du im Konstruktor initialisierst, normalerweise nicht die zu initialisierende Variable rechts vom =-Zeichen steht. Und drittens kommt noch deine Funktion setMatnr dazu. Eine setIrgendwas-Funktion SETZT nochmalerweise einen Memberwert. Deine Funktion setzt aber nichts, sondern holt sich den nächstgrößeren Wert. Finde also einen besseren Namen! Allgemein stellen sich damit 2 Fragen: Warum startet die Funktion nicht gleich bei 1111? Und vor allem: warum autogeneriert der Konstruktor die Matrikelnummer? Ich würde sie einfach auch als Parameter übergeben!

static int index = 0;

Brrr. Vermeide globale Variablen. Die Klasse Verwaltung soll doch verwalten, also sollte diese Klasse auch wissen, wie viele Studenten drin sind.

class Verwaltung:Student

Hiermit sagst du: eine Verwaltung IST EIN Student. Eine Verwaltung hat alle Eigenschaften eines Studenten und kann überall dort verwendet werden, wo ein Student angefragt wird. Das heißt, wenn du eine Funktion bool besuchtVorlestung(Student s) hast, könntest du da eine Verwaltung einsetzen. Das klingt doch logisch falsch! Das hier ist kein Fall von Vererbung!

Ok, hier geht das nicht, weil du privat vererbst (das ist sowieso eine Sache, die du so wenig wie möglich machen solltest). Aber dennoch: BENUTZE Student in der Klasse Verwaltung und erbe nicht davon.

Verwaltung(){
    this->allStud[index];
}

Wenn du keinen std::vector verwenden darfst, musst du dir hier einen nachbauen, wozu du Variablen wie die aktuelle Größe und die aktuelle Kapazität benötigst - oder du machst es die einfach und erlaubst nur eine feste Anzahl an Studenten. Aber bedenke dann: die nach Studentenanzahl größte Uni Deutschlands ist die Fernuni Hagen mit ca. 75.000 Studenten. Ein Array für 100 Studenten, wie hier von @Wade1234 vorgeschalgen, dürfte sowieso für alle realen Unis zu klein sein

Quiche Lorraine

@wob
Was würde der Lehrer wohl sagen, wenn man für die Verwaltung ein std::array nutzen würde?

wob

@Quiche-Lorraine sagte in Programm stürzt nach korrekter Ausgabe ab.:

@wob
Was würde der Lehrer wohl sagen, wenn man für die Verwaltung ein std::array nutzen würde?

Eher: meint der Lehrer wirklich ein Array oder ist vielmehr ein Zeiger gewünscht?

@Sedna schrieb:

Nur ohne "new" auch kein "delete"...

Frage: ist "ohne new und delete" eine Anforderung der Aufgabe oder kommt das von dir?

Sedna

Zuerst einmal: Vielen dank für eure Antworten! Da sind einige Anregungen dabei die mir weiter helfen.

@Wade1234 Ich dachte, dass ich nach jedem neuen Studenten ein um eins größeres Array bekommen würde, da der Index ja erhöht wird...

@DirkB Wenn ich das Programm laufen lasse, werden die beiden Studenten ausgegeben, es returnt aber keine 0.

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

Abgesehen davon ist das Konzept mit den Char-Zeigern falsch.

Spätestens wenn du die Daten mit einer Nutzereingabe holst, fällt dir auf, dass du nur dieselben Daten hast.

Also zeigt beispielsweise mein charpointer *name immer nur auf den ersten Eintrag? Ich dachte, der würde für jedes neue Objekt neu gesetzt werden...
Kannst du mir einen Ansatz geben wie ich's machen müsste?

@manni66 Oh man wie dumm von mir! Da hab ich mich ja mal ordentlich verrannt...

@wob Ja, ich muss es erstmal mit char* machen. Und bei der Sache mit der Matnr. wollte ich es mir einfach machen, damit man nicht jedes mal eine Nr. angeben muss. In der Realität werden die ja auch automatisch vergeben, damit keine doppelt vorkommt. Dass das natürlich nicht ansatzweise so gemacht wird, ist natürlich klar. Dann werde ich es erstmal rausnehmen.

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

Brrr. Vermeide globale Variablen. Die Klasse Verwaltung soll doch verwalten, also sollte diese Klasse auch wissen, wie viele Studenten drin sind.

Das wird immer gepredigt und ich habe es geflissentlich ignoriert! Auch das kommt raus. Kannst du mir aber vielleicht ein Beispiel nennen, bei dem man eine globale Var. verwenden würde?

und dann nochmal zu @wob:

Frage: ist "ohne new und delete" eine Anforderung der Aufgabe oder kommt das von dir?

Ich hatte gelsen, dass man delete nicht nutzen kann, wenn durch new kein Speicher vorher angefordert wurde. In der Aufgabe wurde nämlich darauf hingewiesen, dass man darauf achten soll, dass der Speicher auch wieder freigegeben wird.

Aber okay, dann habe ich wohl noch einiges an Arbeit vor mir. Danke aber nochmal für eure vielen Anregungen. Sowas stärkt die Motivation beim Lernen

DirkB

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

@DirkB Wenn ich das Programm laufen lasse, werden die beiden Studenten ausgegeben, es returnt aber keine 0.

Wie stellst du das fest?
Was erwartest du?
Was passiert?

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

Abgesehen davon ist das Konzept mit den Char-Zeigern falsch.

Spätestens wenn du die Daten mit einer Nutzereingabe holst, fällt dir auf, dass du nur dieselben Daten hast.

Also zeigt beispielsweise mein charpointer *name immer nur auf den ersten Eintrag?

Nein, das hättest du mit deinem Beispiel schon festgestellt.
Du kopierst nur die Zeiger, keinen Inhalt.
Bei deinem Beispiel zeigen die auf die Stringliterale.

Ich dachte, der würde für jedes neue Objekt neu gesetzt werden...
Kannst du mir einen Ansatz geben wie ich's machen müsste?

Du musst beim Anlegen des Studenten dynamisch Speicher für die Namen anfordern. Mit std::string wäre das nicht nötig.

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

@Wade1234 Ich dachte, dass ich nach jedem neuen Studenten ein um eins größeres Array bekommen würde, da der Index ja erhöht wird...

da liegst du aber falsch. es werden lediglich daten an die entsprechende speicherstelle geschrieben, dein programm hat den speicher aber gar nicht zugeteilt bekommen. der aufruf von "this->allStud[index]" ist aber eigentlich gar nichts, bzw. ein zugriff auf das nullte element in dem array. du müsstest da new Student[100] oder sowas aufrufen und im destruktor den speicher mit delete wieder freigeben.

Ich hatte gelsen, dass man delete nicht nutzen kann, wenn durch new kein Speicher vorher angefordert wurde. In der Aufgabe wurde nämlich darauf hingewiesen, dass man darauf achten soll, dass der Speicher auch wieder freigegeben wird.

ja das macht man normalerweise auch so.

Sedna

Hallo ich bin's nochmal,

und zwar habe ich versucht das umzusetzen, was ihr mir geraten habt. Was haltet ihr davon?

class Student{
public:
    Student(){}
    Student(const char name[], const char vorname[], unsigned matnr, unsigned fachsem){
        this->name = name;
        this->vorname = vorname;
        this->matnr = matnr;
        this->fachsem = fachsem;
    }

private:
    const char* name;
    const char* vorname;
    unsigned matnr;
    unsigned fachsem;
};

class Verwaltung{
public:
    Verwaltung(){
        allStud = new Student[kapa];
    }
    ~Verwaltung(){
        delete[] allStud;
        allStud = 0;
    }

    newStud(const char name[], const char vorname[], unsigned matnr, unsigned fachsem){
        if(index<kapa){
            Student stud(name,vorname,matnr,fachsem);
            allStud[index] = stud;
            ++index;
        }else{
            help = new Student[kapa+=100];
            for(int i=0; i<index; ++i){
                help[i] = allStud[i];
            }
            allStud = help;
            Student stud(name,vorname,matnr,fachsem);
            allStud[index] = stud;
            ++index;
        };
    }

private:
    Student *allStud;
    Student *help;
    int index = 0;
    int kapa = 100;
};

Jetzt bin ich mir aber extrem unsicher was der Destruktor so treibt. Um das nachzuvollziehen hat der noch ein cout bekommen. Zusätlich habe ich die Größe des Array auf 1 gesetzt und lasse dies auch nur mit 1 "vergrößer". Dann drei neue Studenten angelegt. Der Destruktor hat sich nur einmal zu Wort gemeldet... hätte es nicht zweimal, oder mehr sein müssen? Und ist der Schlüssel dazu mit einer Schleife über das Array zu laufen und dann alles einzeln zu löschen?

wob

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

Der Destruktor hat sich nur einmal zu Wort gemeldet...

Du erzeugst zwar das neue Studentenarray hier help = new Student[kapa+=100]; (bitte keine 2 Befehle zusammen! das kapa-Erhöhen und das new sollten je in einer Zeile sein) und dann überschreibst du den alten Pointer hier: allStud = help;. Damit kennt niemand mehr dein altes, kleineres Array. Und somit wird es niemals gelöscht. Also müsstest du hier erst das alte Array löschen: delete[] allStud; allStud = help;.

Wie du siehst, ist es EXTREM fehleranfällig, mit new/delete zu arbeiten, deswegen raten wir hier immer davon ab (eine Frage ist z.B., was passiert, wenn du dein Verwaltungsobjekt kopierst oder was passiert).

Im übrigen sollte Student *help keine Member-Variable sein, sondern nur innerhalb der Funktion definiert werden. Also gleich mit kapa += 100; Student *help = new Student[kapa];.

Du könntest zum Beispiel dein Student *allStud durch std::unique_ptr<Student[]> allStud ersetzen, dann kannst du mit allStud = std::make_unique<Student[]>(kapa) dir ein Smartpointer holen (für den du kein delete brauchst). Aber viel besser wäre ein std::vector<Student>. Dir wird auch noch aufgefallen sein, dass du beim Erhöhen der Kapazität jeweils "leere" Studentenobjekte erzeugst, die du für nichts gebrauchen kannst. Das wäre mit vector auch nicht der Fall.

SeppJ

Falls du zusätzlich zu dem Schrott, den dir dein Lehrer beibringt, etwas Nützliches lernen möchtest: Dies ist ein ganz ausgezeichnetes Beispiel für Trennung von Zuständigkeiten. Das ist ein wichtiges Prinzip für gutes Klassendesign (im Gegensatz zu dem Design, dass dir hier vorgeschlagen wird). Es bedeutet, dass eine Klasse genau eine Sache tun sollte und nichts anderes.

Konkret bedeutet das hier: Deine Stundentenklasse (und deine Verwaltungsklasse hat genau das gleiche Problem) tut zwei Dinge: Sie stellt einen Studenten dar, aber sie verwaltet auch Zeichenketten. Wie viele Studenten kennst du im echten Leben, die Zeichenketten verwalten?

Besser: Trenne das in zwei Klassen: Eine, die einen Studenten darstellt, und eine weitere, die Zeichenketten verwaltet. Die Zeichenkettenverwaltungsklasse kümmert sich um die ganze Speicherverwaltung einer Zeichenkette und macht dies (im Gegensatz zu deinen jetzigen Versuchen) richtig. Die Studentenklasse benutzt dann diese Zeichenkettenklasse. Da in beiden Fällen die Aufgabenstellung der Klasse kleiner und klar abgegrenzt ist, ist es viel einfacher, die jeweiligen Aufgaben der Klasse korrekt zu implementieren und zu testen.

Das ist zwar nicht genau der Wortlaut der Aufgabenstellung, aber nahe genug dran, und in jedweder Hinsicht um so vieles besser als das vorgeschlagene Design, dass jeder vernünftige Lehrer dich loben würde, wenn du das so machst.

Wenn du es noch schöner machen möchtest, erkennst du natürlich, dass Speicherverwaltung auch nicht die Kernaufgabe einer Zeichenkette ist. Stattdessen wäre es angebracht, noch zusätzlich eine allgemeine Speicherverwaltungsklasse zu schreiben, die von der Zeichenkette genutzt wird. Und günstigerweise brauchst du eine allgemeine Speicherverwaltung auch noch für die Stundentenverwaltung. Das heißt, du kannst so eine Speicherverwaltungsklasse gleich mehrmals gut anbringen. Wenn du die Speicherverwaltung nur ein einziges mal in dieser Klasse korrekt implementierst, dann werden alle deine anderen Klassen nahezu trivial, und du sparst noch Unmengen an Code, der sich ansonsten zigmal nahezu identisch in deinem Programm wiederholt hätte.

Wenn du deinem Lehrer noch so richtig klarmachen möchtest, wie schlecht die Aufgabenstellung ist, dann nennst du die Speicherverwaltungsklasse vector und die Zeichenkettenklasse string. Aber wahrscheinlich würde er die Anspielung nicht einmal verstehen

und du musst kapa erhöhen, sonst hast du nach 100 studenten ein problem. das ist ein sehr schönes beispiel für die vorzüge von vektoren bzw. den standardfunktionalitäten allgemein, aber wenn du eben lernen sollst, wie das ganze zeugs intern abläuft, wie es in alten programmen aussehen könnte usw........ edit: und help musst du auch noch auf 0 (oder besser nullptr) setzen.

Sedna

Danke, dass ihr euch die Zeit dafür nehmt.

newStud() sieht nun so aus:

newStud(const char name[], const char vorname[], unsigned matnr, unsigned fachsem){
        if(index<kapa){
            Student stud(name,vorname,matnr,fachsem);
            allStud[index] = stud;
            ++index;
        }else{
            Student *help;
            kapa += 100;
            help = new Student[kapa];
            for(int i=0; i<index; ++i){
                help[i] = allStud[i];
            }
            delete[] allStud;
            allStud = help;
            help = 0;
            Student stud(name,vorname,matnr,fachsem);
            allStud[index] = stud;
            ++index;
        };
    }

Frage an @wob: Warum nicht help = new Student[kapa+=100]? Einfach der Übersicht halber, oder könnte das noch andere Probleme verursachen, weil nicht klar ist was zuerst passiert? Und brauche ich nun keinen selbst geschriebenen Destruktor mehr, weil newStud() den Speicher wieder freigibt?

Und an @Wade1234: kapa wird doch in Zeile 8 erhöht.

Swordfish

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

Warum nicht help = new Student[kapa+=100]? Einfach der Übersicht halber.

Ja. Geschmackssache. Die Auswertungsreihenfolge ist auch in Deiner ursprünglichen Version klar definiert und würde passen.
Was aber so oder so ein Problem ist, ist, daß Du kapa veränderst obwohl sowohl new als auch der Konstruktoraufruf deiner Elemente werfen können. Wenn das passiert hast Du eine Verwaltung deren kapa nicht mehr stimmt.

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

Und brauche ich nun keinen selbst geschriebenen Destruktor mehr, weil newStud() den Speicher wieder freigibt?

Denk' nochmal nach was für einen Speicher dein newStud() freigibt.

Überlege Dir bitte ernsthaft den Vorschlag von @SeppJ. Da lernst Du mehr.

Sedna

@Swordfish Ich verstehe worauf ihr hinaus wollt. In den späteren Übungen werden Vektoren etc ja auch noch behandelt. Für die Prüfung möchte ich einfach alles nochmal durchkauen, was der Prof so von uns wollte. Und der Lerneffekt ist bei dieser Sache ja auch nicht ausgeblieben.

Und an @Wade1234: kapa wird doch in Zeile 8 erhöht.

also in dem programm, das du (laut forensoftware) vor 4 stunden gezeigt hast, wurde kapa überhaupt nicht erhöht. ansonsten würde ich dir dringend empfehlen, irgendwie bücher über softwareengineering und objektorientierte programmierung zu lesen und durchzuarbeiten; das ist viel wichtiger, als c++ in allen einzelheiten zu kennen, bzw. "veraltetes" aber funktionierendes c++ ist besser, also "modernes" aber fehlerhaftes c++.

wob

Doch sicher wurde es erhöht! Hier: help = new Student[kapa+=100];. Und das ist auch genau der Grund, warum man das nicht in einer Zeile machen sollte: man übersieht es einfach.

wob

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

edit: und help musst du auch noch auf 0 (oder besser nullptr) setzen.

Warum?

Siehe auch meinen vorherigen Kommentar zu dieser Variable.

Swordfish

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

edit: und help musst du auch noch auf 0 (oder besser nullptr) setzen.

Ja. Ist idiomatischer Unfug.

naja es kostet eben 2 taktzyklen, den nullzeiger zu setzen, aber verhindert evtl. ganz furchtbare programmfehler, weil das programm dann ganz sicher (100%) abstürzt, weshalb es sich lohnt, sich das setzen von nullzeigern anzugewöhnen. also soweit ich weiß......

edit: achja statt += in der eckigen klammer sollte man den wert der variable eine zeile vorher erhöhen. der compiler kann jeden noch so primitiven code optimieren, aber es leidet zumindest die lesbarkeit bzw. verständlichkeit darunter und manchmal versteht der compiler es auch nicht. also meiner erfahrung nach kann der compiler am besten optimieren, wenn du den code "maximal doof" programmierst, also für jede noch so blöde berechnung eine eigene ergebnisvariable erstellt wird usw.. das hat auch den vorteil, dass du mit einfachem copy paste codeteile von a nach b bewegen kannst, ohne dass da probleme auftauchen. aber der compiler optimiert es eben, wenn im quelltext 10000 mal hintereinander die gleiche berechnung durchgeführt wird.

Swordfish

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

weil das programm dann ganz sicher (100%) abstürzt, weshalb es sich lohnt, sich das setzen von nullzeigern anzugewöhnen. also soweit ich weiß......

Ja. Ähm. Nee. Sowas crasht in der Regel auch beim Zugriff auf bereits freigegebenen Speicher wenn die Runtime nicht ganz Banane ist.

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

aber der compiler optimiert es eben, wenn im quelltext 10000 mal hintereinander die gleiche berechnung durchgeführt wird.

Das ist so 'ne Sache. In

for (auto it = foo.begin(); it != foo.end(); ++it)
    // ... whatever.

muss der Compiler eben nachweisen können daß sich foo.end() im Schleifenkörper nicht ändern tut.

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

In den späteren Übungen werden Vektoren etc ja auch noch behandelt.

Davon hat @SeppJ nicht geredet. Selber implementieren, nicht std::vector<> nutzen.