Speicherverletzungen

Hi liebe Community,

Ich bin relativ neu bei C++ und habe programmieren mit Java begonnen.

Nun habe ich eine Aufgabe bekommen und erhalte bei Ausführen des Programms, wenn ich Methoden öfter aufrufe Speicherverletzungen. Ich habe absichtlich die Pointer vermieden um dieses Problem zu umgehen und weiß nun nicht was ich falsch gemacht habe. Eventuell könntet ihr mal über meine main-Klasse drüber schaun und mir einen Tipp geben.

http://pastebin.com/rFTN2mGq

Was ist der grundsätzliche Fehler bei meinem Programm?

Hier noch das UML:

http://imageshack.us/photo/my-images/207/uml.png/

In meinen Objekt-Klassen gibt es eigtl. nur getter und setter sowie manchmal eine toString Methode.
Ich bin um jede Hilfe dankbar!

314159265358979

Nimm einen Debugger.

redrew99

In Zeile 178-191 steht:

void createMediaLibrary(){
	MediaLibrary m;

	cout << "Create a media library" << endl;
	cout << "=====================" << endl;
	cout << "Enter the name of the library: ";
	string s;
	cin >> s;
	m.setName(s);

	cout << "Done! You have succesfully created a new media library!" << endl;

	_vMediaLibrary.push_back(m);
}

Meiner Ansicht nach ist der Umgang mit dem Vektor _vMediaLibrary nicht gut gelöst,
besser wäre evtl. eine eigene Funktion, die den Vektor und das Objekt als Parameter (Zeiger oder Referenz) übergibt.

Du hast am Anfang des Programms diverse Vektoren mit globalem Gültigkeitsbereich
erzeugt, das ist glaube ich kein guter Programmierstil(mal gucken ob ich gleich gesteinigt werde... ), sofern es Alternativen dazu gibt.

otze

AudioTrack* track = (AudioTrack*) vTrack[i];
cout << ss.str() + ": " + track->toString() << endl;
delete track;

Das sieht nicht so aus, als wenn es das täte, was du willst.

krümelkacker

Ich hab's jeztt nur überflogen…

3mal "Ihhh!!":

globale Objekte
C-style Casts
Speichern von Zeigern auf Elemente von Vektoren die sich eventuell vergrößern

Die C-style Cast Syntax ist zwar in Java üblich, bedeutet da aber etwas anderes. In C und C++ bedeutet das: "Hey, Compiler! Ich weiß es besser als Du und Du musst auch nichts überprüfen. Vertrau mir einfach und halt die Schnauze!". Wenn Track und AudioTrack, VideoTrack in einer Vererbungshierarchie stecken und Track polymorph ist (wofür du selbst sorgen musst, das wird dir nämlich nicht wie bei Java aufgezwungen), dann ist ein dynamic_cast viel passender -- wobei ein dynamic_cast eigentlich recht seltend passend ist. Vermeide so'was einfach durch ein ggf besseres Design…

Wenn Vektoren sich vergrößern, werden Objekte eventuell umkopiert. Dadurch werden natürlich Zeiger auf die alten Objekte ungültig, weil die neuen ganz woanders im Speicher stehen werden.

Über die tendentielle Schlechtigkeit von globalen Objekten muss ich hoffentlich nichts mehr sagen.

Tipp am Rande: Die Unterschiede zwischen Java und C++ werden von Dir wahrscheinlich unterschätzt. Ich hoffe, Du hast Dir ein schlaues C++ Buch besorgt. Ich habe auch Java vor C++ gelernt. Deswegen kenne ich das schon…

Cheers!
kk

Mein Problem war, dass ich die Objekte alle so erstellen muss:

MediaLibrary* m = new MediaLibrary();

Da ich jetzt alles auf Pointer umgebaut hab, funzt das so.

otze schrieb:

AudioTrack* track = (AudioTrack*) vTrack[i];
cout << ss.str() + ": " + track->toString() << endl;
delete track;
Das sieht nicht so aus, als wenn es das täte, was du willst.

Doch, das tut es. Da ich für die Klasse Track keine eigene toString() habe, sondern nur für AudioTrack und VideoTrack muss ich die Objekte erst casten und dann die gewünschte toString() aufrufen. Hätte man sicherlich eleganter lösen können. (dynamic_cast) so wie krümel das schon gesagt hat.

krümelkacker schrieb:

Ich hab's jeztt nur überflogen…

Über die tendentielle Schlechtigkeit von globalen Objekten muss ich hoffentlich nichts mehr sagen.

Tipp am Rande: Die Unterschiede zwischen Java und C++ werden von Dir wahrscheinlich unterschätzt. Ich hoffe, Du hast Dir ein schlaues C++ Buch besorgt. Ich habe auch Java vor C++ gelernt. Deswegen kenne ich das schon…

Cheers!
kk

Leider habe ich noch kein Buch besorgt, werde ich vl noch machen, wenn ich wieder mit C++ was zu tun hab. Da ich aber Mobile Computer bin, werd ich mich wschl in Zukunft sowieso mehr auf Android und iOS konzentrieren. Aber das Fach muss ich ja trotzdem positiv abschließen.

Globale variablen in diesem Fall schlecht, weil? Würde es schonmal besser sein diese private zu machen?

Lg

Skym0sh0

globale variablen sind aus folgenden gründen schlecht:

- sie können von überall geändert werden, da kann man gut den überblick verlieren, wann das passiert -> wann sich fehler einschleichen
- sie nutzen die komplette programmlaufzeit speicher, obwohl sie vielleicht gar nicht gebraucht werden
- sie schaffen weitere abhängigkeiten zwischen kompilierungseinheiten (du inkludierst in headerA.h eine headerB.h datei, in headerA wird aber ein globales objekt benutzt/gebraucht, welches erst in headerB definiert wird)

- to be continued

4ndro1d schrieb:

otze schrieb:
AudioTrack* track = (AudioTrack*) vTrack[i];
cout << ss.str() + ": " + track->toString() << endl;
delete track;
Das sieht nicht so aus, als wenn es das täte, was du willst.
Doch, das tut es. Da ich für die Klasse Track keine eigene toString() habe, sondern nur für AudioTrack und VideoTrack muss ich die Objekte erst casten und dann die gewünschte toString() aufrufen. Hätte man sicherlich eleganter lösen können. (dynamic_cast) so wie krümel das schon gesagt hat.

nein, das tut es nicht. was du machst ist ein downcast, dafür ist eigentlich dynamic_cast da, und eine fallunerscheidung, weil dieser cast auch schiefgehen kann. ok, das klappt aber hier.
das ding ist nur das delete danach.

du hast einen vector, wo ein objekt drin ist. das objekt holst du dir mit vTrack[i], ok. aber dann löschst du das. d.h. du entziehst dem, der die controlle hat über die speicherverwaltung (also dem vector, der das am ende selbst löschen würde), eben die kontrolle, indem du ein objekt löscht. das es klappt bei dir ist wiederum glück, vielleicht implodiert ja das universum wenn ich dein programm auf meinem rechner ausführen lasse. warum? weil das ziemlich gut undefined behaviour sein könnte...

http://pastebin.com/rM9u3Euf

So läuft jetzt alles wie ich es will
Also sollt ich die ganzen Methoden noch in ne extra Klasse packen incl. der globalen Variablen und dann in der main nur über getter/setter zugreifen?
(Werd ich in diesem Fall aber eh nimmer machen)

Achja und wie funktioniert denn das mit dem dynamic_cast dann?
Das muss in der Klasse Track angegeben werden?

Skym0sh0

cass Base
{
public:
    std::string toString();
    virtual ~Base();
};

class Derive : public Base
{
public:
    std::vector<char> toCharArray();
};

int main()
{
    Base * b = new Derive;  // alles kein problem, upcast implizit
    b.toString(); // alles legal
    b.toCharArray(); nicht legal, da b ein Base ist. Und das obwohl es eigentlich ein Derive ist!! Also vorsicht...

    // Derive * d = (Derive*)(&b); // war deine möglichkeit, aber das ist mies

    Derive * d = dynamic_cast<Derive>(&b);
    if ( !d ) // wen der cast fehlschlägt, zeigt d auf NULL -> nullptr
        std::cout << "Cast Failed!" << std::endl;

    d.toCharArray();

    delete b; // dran denken, wenn du mit pointern rummachst, aber nicht d UND b löschen! eins von beiden reicht, und da du b die speicher rechte quasi gegeben ahst oben mit new, mach es auch über b wieder sauber

    return 0x0;
}

krümelkacker

4ndro1d schrieb:

Achja und wie funktioniert denn das mit dem dynamic_cast dann?

Wie das funktioniert? Ich nehme mal an, dass der Compiler Code erzeugt, der zur Laufzeit Metadaten überprüft, die der Compiler für polymorphe Klassen anlegt. Wie genau das funktioniert ist nicht interessant. Interessant ist die Syntax und die Regeln dazu und das findet man ganz schnell im Netz für lau … oder schön erklärt in einem guten Buch.