Stack-Implementierung

[icarus2]

Was meinst du mit "er greift daneben"? Das Problem liegt in Zeile 51. Du hast da

//fügt bei jedem Aufruf ein neues "int" hinzu
void stack::stackInit(int counter)
{
    int *feld = new int[counter] ();
    i = counter;
}

Das muesste lauten

feld = new int[counter];

sonst initialisierst du nicht die Klassenvariable, sondern bloss eine lokale Variable.

Verwende ausserdem einen std::vector fuer einen dynamisch grossen Stack oder std::array fuer einen Stack konstanter Groesse. Nach den heutigen Software-Engeneering Principles sollte der Stack dynamisch sein - ausser man hat einen sehr guten Grund den Stack nicht dynamisch zu machen.

Die Arbeit der init-Methode sollte besser im Konstruktor erledigt werden. Genau dazu ist er da.

[vip*r]

@icarus2:

Den Fehler hab ich mittlerweile Dank SeppJ gefunden

@SeppJ: Compilerwarnungen hab ich eingeschaltet. Er hat nicht gemeckert. Und ja das ist nur eine Übung und ich will nicht die Stack-Bib nachcoden. So geht's jetzt aber:

//fügt bei jedem Aufruf ein neues "int" hinzu
void stack::stackInit(int counter)
{
	feld = new int[counter] ();
	i = counter;
}

Designmängel? Kannst du näher drauf eingehen? Ich hab ehrlich gesagt nicht gewusst, was ich in den Konstruktor reinschreiben soll! Natürlich weiß ich, dass man da die eigentlich Initialisierung macht... Aber wie sieht das konkret hier aus?

[arghonaut]

Ein anderes Problem:
Ein Stack ist ein Stapel, auf den man Sachen legen kann, und der dabei wächst, nicht ein magisches Objekt, das eine einzige Sache schweben lassen kann.
Schau mal genau an, wie du i benutzt, und was das für einen Effekt in push() hat.

[Bashar]

vip@r schrieb:

while(eingabeWert != 0)
	{
		myStack.stackInit(i);
		i++;
		cout << "Wert eingeben: ";
		cin >> eingabeWert;

		myStack.push(eingabeWert);
	}

Bau deinen Stack so, dass man ihn folgendermaßen benutzen kann:

while(eingabeWert != 0)
	{
		cout << "Wert eingeben: ";
		cin >> eingabeWert;

		myStack.push(eingabeWert);
	}

Deine stackInit-Funktion ist ein einziger Misthaufen. Sie wirft den Stackinhalt weg, erzeugt Memoryleaks, hat einen irreführenden Namen und ist technisch völlig unnötig. Weg damit.

[Michael E.]

icarus2 schrieb:

Verwende ausserdem einen std::vector fuer einen dynamisch grossen Stack oder std::array fuer einen Stack konstanter Groesse.

http://www.cplusplus.com/reference/stl/stack/

[vip*r]

@Bashar: Wenn ich den Stack ohn init() bauen soll, muss ich die initialisierung im Konstruktor machen. Wie mach ich das dann, dass er mir vor jedem Aufruf der push-Methode ein neues int-Anordert und genau hinter dem "alten" int anordnet? Das ist nämlich ehrlich gesagt das Problem warum ich das nicht gleich im Konstruktor gemacht habe...

Wie geht das?

[SeppJ]

vip@r schrieb:

Designmängel? Kannst du näher drauf eingehen? Ich hab ehrlich gesagt nicht gewusst, was ich in den Konstruktor reinschreiben soll! Natürlich weiß ich, dass man da die eigentlich Initialisierung macht... Aber wie sieht das konkret hier aus?

Die schwerwiegendsten Sachen kannst du in meiner ersten Antwort in deinem letzten Thread ganz ausführlich und am Beispiel erklärt nachlesen. Die Antworten gelten hier wie da. Da du es das letzte Mal schon nicht gelesen hast, habe ich keine Lust, die nochmals so eine lange Antwort zu geben. Daher Stichpunkte:

- Member inputVar total nutzlos.
- Regel der großen Drei (wieder mal) verletzt.
- Leerer Konstruktor, wozu dann überhaupt einen Konstruktor?
- Init-Methode? Du meinst wohl Konstruktor!
- Mit new[] erzeugt, mit delete gelöscht. Das mag bei int zwar gutgehen, ist aber trotzdem falsch.
- Was passiert, wenn init mehrmals aufgerufen wird? (Wenn init ein Konstruktor wäre, dann würde sich die Frage gar nicht stellen)
- (reverse)print ist mal wieder das gleiche wie ein operator<<, bloß mit ungewohntem Namen.
- Bei einem Stack brauchst du doch auch so etwas wie einen Stackpointer, oder? Du meinst hier vermutlich i, aber das passt nicht, so wie du es benutzt.
- Siehe vorhergehender Punkt: Was für ein nichtssagender Name ist i?

[Bashar]

vip@r schrieb:

@Bashar: Wenn ich den Stack ohn init() bauen soll, muss ich die initialisierung im Konstruktor machen. Wie mach ich das dann, dass er mir vor jedem Aufruf der push-Methode ein neues int-Anordert und genau hinter dem "alten" int anordnet? Das ist nämlich ehrlich gesagt das Problem warum ich das nicht gleich im Konstruktor gemacht habe...

Das ist noch zu kompliziert für dich. Du solltest es erstmal einfacher versuchen. Da bieten sich mehrere Varianten an:

1. Du benutzt einen Container, der beim Einfügen mitwächst. Das heißt deine Daten liegen in einem std::vector, du baust nur das push/pop-Interface drumherum. Das ist ziemlich trivial, und auch ziemlich genau das, was std::stack macht, aber vielleicht hast du ja Lust.

2. Du baust dir einen Stack mit fester Kapazität, die im Konstruktor festgelegt wird. Beim push guckst du, ob noch Platz ist, wenn nein wirfst du eine Exception.

3. Du benutzt eine verkettete Liste, an deren Anfang oder Ende du die neuen Objekte anfügst.

[vip*r]

[quote="SeppJ"]

vip@r schrieb:

- Member inputVar total nutzlos.
- Regel der großen Drei (wieder mal) verletzt.
- Leerer Konstruktor, wozu dann überhaupt einen Konstruktor?
- Init-Methode? Du meinst wohl Konstruktor!
- Mit new[] erzeugt, mit delete gelöscht. Das mag bei int zwar gutgehen, ist aber trotzdem falsch.
- Was passiert, wenn init mehrmals aufgerufen wird? (Wenn init ein Konstruktor wäre, dann würde sich die Frage gar nicht stellen)
- (reverse)print ist mal wieder das gleiche wie ein operator<<, bloß mit ungewohntem Namen.
- Bei einem Stack brauchst du doch auch so etwas wie einen Stackpointer, oder? Du meinst hier vermutlich i, aber das passt nicht, so wie du es benutzt.
- Siehe vorhergehender Punkt: Was für ein nichtssagender Name ist i?

inputVar nutzlos? Wie soll ich denn dann den von cin kommenden Wert in des feld[] schreiben?
Was sind die großen Drei?
Hilft mir hier eine Konstruktorliste?
Hab schon verstanden, ich soll die init wegschmeißen und das dem konstruktor überlassen....

[cooky451]

vip@r schrieb:

inputVar nutzlos? Wie soll ich denn dann den von cin kommenden Wert in des feld[] schreiben?

Und wo machst du irgendwann mal etwas mit der Membervariable inputVar?

vip@r schrieb:

Was sind die großen Drei?

Google?

[vip*r]

#include<iostream>
using namespace std;

class stack
{
private:
	int i;
	int *feld;
	int inputVar;

public:
	stack();
	~stack();

	void push(int inputVar);
	void reversePrint();
};

//Konstruktor
stack::stack()
{
	feld = new int[1000];
	i = 0;
}

//Destruktor
stack::~stack()
{
	delete feld;
}

void stack::push(int inputVar)
{
	feld[i] = inputVar;
	i++;
}

void stack::reversePrint()
{
	for(int counter=0; counter<i; counter++)
	{
		cout << feld[counter];
	}
}

int main()
{
	int eingabeWert = 1;
	stack myStack;

	while(eingabeWert != 0)
	{
		cout << "Wert eingeben: ";
		cin >> eingabeWert;

		myStack.push(eingabeWert);
	}

	myStack.reversePrint();

return 0;
}

In Zeile 38 weise ich dem Feld den Wert aus inputVar zu. Wie ich das jetzt auch noch weglassen soll, verstehe ich nicht. Jetzt hab ich übrigens den Konstruktor geschrieben und init weggelassen. Des Weiteren hab ich mich erstmal für eine feste allokierte Größe entschieden.

[cooky451]

vip@r schrieb:

In Zeile 38 weise ich dem Feld den Wert aus inputVar zu. Wie ich das jetzt auch noch weglassen soll, verstehe ich nicht.

Niemand redet von dieser Zeile. Diese Variable ist ja auch ein Paremter und keine Membervariable, ordentlich lesen sollte man schon. Zeile 10 sollst du weglassen.

[SeppJ]

Guck nochmal genau hin. Such mal in deinem Editor nach inputVar und denk darüber nach, welches inputVar du wohl benutzt.

- Was passiert, wenn i > 999 wird?
- i ist immer noch ein dummer Name.
- Regel der großen 3 immer noch nicht befolgt. (Ok, das wäre auch ein bisschen viel Änderung in der kurzen Zeit)
- Immer noch mit new[] angelegtes Feld mit delete gelöscht. Das wäre nur eine kleine Änderung. Weißt du überhaupt, warum das falsch ist?
- Im Konstruktor könnte und sollte eine Initialisierungsliste benutzt werden. Der Code macht dann zwar letztlich das gleiche, spart sich aber ein paar Umwege. Und es übt für die Fälle, wenn man eine Initialisierungsliste benutzen muss (ja, solche Fälle gibt's oft, auch wenn man das als Anfänger vielleicht noch nicht sieht)
- Der Rest ist besser geworden.

[vip*r]

Ein weiterer Versuch:

#include<iostream>
using namespace std;

class stack
{
private:
	int memberVar;
	int *feld;

public:
	stack();
	~stack();

	void push(int inputVar);
	void reversePrint();
};

//Konstruktor
stack::stack() : memberVar(0)
{
	feld = new int[1000];
}

//Destruktor
stack::~stack()
{
	delete feld;
}

void stack::push(int inputVar)
{
	feld[memberVar] = inputVar;
	memberVar++;
}

void stack::reversePrint()
{
	for(int counter=memberVar-1; counter>=0; counter--)
	{
		cout << feld[counter];
	}
}

int main()
{
	int eingabeWert = 1;
	stack myStack;

	while(eingabeWert != 0)
	{
		cout << "Wert eingeben: ";
		cin >> eingabeWert;

		myStack.push(eingabeWert);
	}

	myStack.reversePrint();

return 0;
}

- Was passiert wenn i>999 wird? Dann gibts einen Fehler weil das angeforderte Array zu klein ist. Ich hab ein paar Antworten früher gesagt, dass ich mich für die festvorgegebene Größe entschieden habe. Wenn ich es absolut dynamisch haben möchte, müsste ich mit "stack" aus der STL arbeiten. Das werde ich auch noch ausprobiere
- Zeile 10 entfernt; und sie war in der Tat überflüssig
- i hab ich in memberVar umbenannt
- Konstruktorliste hinzugefügt; warum man aber das machen soll, ist mir ehrlich gesagt nicht ganz klar
- Die Regel der großen Drei hab ich nachgelesen (aber noch nicht umgesetzt). Anscheinend braucht man um diese Regel zu erfüllen auch einen Copy-Konstruktor. Wie realisiert man sowas und was würde der dann eigentlich in diesem Programm machen?
- Warum es falsch ist, mit new[] zu erzeugen und mit delete zu löschen, weiß ich nicht, vor allem weil es ja eben auch funktioniert. Kannst du mich aufklären?

Ist es jetzt wieder ein Stückchen besser?

[Tachyon]

vip@r schrieb:

- Was passiert wenn i>999 wird? Dann gibts einen Fehler weil das angeforderte Array zu klein ist.

Ne, den gibt es nicht. Es passiert irgendein Scheiss - im schlimmsten Fall läuft das Programm in halbgarem Zustand weiter ohne dass Du es sofort merkst. Einen Fehler gibt es nur, wenn Du selbst dafür sorgst, dass es einen gibt.

vip@r schrieb:

- i hab ich in memberVar umbenannt

Ist auch Mist. Nenne Variablen so, dass man erkennt, wozu sie benutzt werden. Diese hier wird nicht zum "memberVaren" benutzt, sondern sie speichert die Einfügebposition in den Stack. "insertPosition" o.ä. wäre hier also ein besserer Name.

vip@r schrieb:

- Konstruktorliste hinzugefügt; warum man aber das machen soll, ist mir ehrlich gesagt nicht ganz klar

Da gibt es diverse Gründe. Einige Dinge kannst Du anders gar nicht initialisieren. Basisklassen ohne Default-Ctor zum Beispiel. Außerdem ist es für komplexe Datentypen (aka Klassen) performanter, es so zu machen. Generell ist das der Weg Membervariablen zu initialisieren.

vip@r schrieb:

- Die Regel der großen Drei hab ich nachgelesen (aber noch nicht umgesetzt). Anscheinend braucht man um diese Regel zu erfüllen auch einen Copy-Konstruktor. Wie realisiert man sowas und was würde der dann eigentlich in diesem Programm machen?

Wenn Du die Regel nachgelesen hast, solltest Du vielleicht auch gleich nachlesen, was der Copy-Ctor macht, wenn Du es nicht weisst. Und zur Regel gehört noch mehr als nur der Copy-Ctor. Also alles Lesen und dann selbst tätig werden.

vip@r schrieb:

- Warum es falsch ist, mit new[] zu erzeugen und mit delete zu löschen, weiß ich nicht, vor allem weil es ja eben auch funktioniert. Kannst du mich aufklären?

Zu einem new gehört ein delete und zu einem new[] gehört ein delete[] . Das wurde Dir an anderer Stelle auch schon gesagt.

vip@r schrieb:

Ist es jetzt wieder ein Stückchen besser?

Nein.