Iteratoren - Stack als Klassentemplate - die zweite
-
Auf den Fehler habe ich und camper dich bereits hingewiesen. (Auch wenn ich bei mir das Template Argument vergessen habe).
Im übrigen solltest du die Kommentare, die camper dir gegeben hat dennoch mal anschauen, wenn er sich schon die Mühe macht alles so schön zu kommentieren.
-
drakon schrieb:
Auf den Fehler habe ich und camper dich bereits hingewiesen. (Auch wenn ich bei mir das Template Argument vergessen habe).
Im übrigen solltest du die Kommentare, die camper dir gegeben hat dennoch mal anschauen, wenn er sich schon die Mühe macht alles so schön zu kommentieren.
ich habe die Kommentare von camper sehr wohl durchgelesen, ...
camper schrieb:
// Stack.h // #ifndef STACK_H_ #define STACK_H_ class StackError { // Exceptionklasse public: char *what(void); // dies sollte alles mit const char* StackError(char *); // arbeiten, da du hier vorhast, Literale private: // zu übergeben, zudem ist kein sinnvoller Grund erkennbar char *error; // weshalb ein Handler den String verändern wollte }; StackError::StackError(char *error) { // bevorzuge die Initialisierungsliste this->error = error; // ist hier zwar egal, kann aber sonst schnell unübersichtlich werden } char *StackError::what(void) { // Abfrage um welche Exception es sich handelt // Funktion verändert das Exception-Objekt nicht -> const return error; } // StackError ist simpel genug, dass es kürzer und verständlicher wird, wenn du die Funktionenn inline in der Klassendefinition definierst
leider alles vorgaben meines profs.
camper schrieb:
template <class T> class Stack { private: struct list_el; list_el *stack_pointer; public: Stack(); ~Stack(); void push(const T &) throw (StackError); // die Exceptionspezifikation ist zweifelhaft (hängt tatsächlich von T ab) void top(T &) throw (StackError); // dito, gibt logisch einen Wert zurück, für Spielereien mit Referenzen gibt es keinen Grund // --> const T& top() const oder T top() const // in ersterem Falle müsste man spezifizieren, ob und wann die Referenz ungültig wird // Funktion verändert den Stack nicht -> const void pop() throw (StackError); bool is_empty(); // -> const class iterator { // iterator-Klasse
ebenfalls alle funktionsrümpfe vorgaben meines profs.
camper schrieb:
public: iterator(list_el *pos = NULL); void operator++() throw (StackError); // postfix ++ fehlt void operator+=(const int) throw (StackError); // operator + fehlt (sollte aber freie Funktion bleiben -> ggf. friend oder Bezug auf +=) void insert() throw (StackError); // was macht diese Funktion ? void erase() throw (StackError); // dito private: list_el *pElement; // Zeiger auf Element }; iterator begin(); // -> const iterator end(); // -> const };
sollten nur den präfix ++-operator machen sowie den +=, ebenfalls vorgaben des profs. die funktion erase loescht ein element vom stack, auf das der iterator zeigt, insert fügt eins ein. vorgabe vom prof, und ja ich weiß, dass das den sinn und zweck eines lifo widerspricht.
camper schrieb:
////////////// LISTENELEMENTE ////////////// template <typename T> // Template fuer Listenelement struct Stack<T>::list_el { list_el(list_el *pnext, const T &data) : pnext(pnext), data(data) {} // Konstruktor list_el *pnext; // Verkettung T data; // Daten des Listenelements }; ////////////// STACK ////////////// template <class T> Stack<T>::Stack(void) { // Initialsierungsliste ? stack_pointer = NULL; } template <class T> void Stack<T>::push(const T &element) throw (StackError) { // schiebt Element in den Stack stack_pointer = new list_el(stack_pointer, element); // erstellt neues Listenelement if(stack_pointer == NULL) // wenn kein Speicher vorhanden ... // das kann niemals passieren throw StackError("kein Heapspeicher frei!"); // Exception // ^ } template <class T> void Stack<T>::top(T &element) throw (StackError) { // liest erstes Element if(is_empty()) throw StackError("Stack ist leer!"); element = stack_pointer->data; // gibt erstes Element zurueck } template <class T> void Stack<T>::pop(void) throw (StackError) { // loescht erstes Element if(is_empty()) throw StackError("Stack ist leer!"); list_el *temp = stack_pointer; // temp zum loeschen des Elements stack_pointer = stack_pointer->pnext; delete temp; temp = NULL; // überflüssig
nicht überflüssig, sonst wird das element auf den der stack_pointer gezeigt hat nicht entfernt
camper schrieb:
} template <class T> bool Stack<T>::is_empty(void) { if(stack_pointer == NULL) // prueft ob erstes Element vorhanden // bitte return true; // nicht else // so return false; // umständlich
wie weniger umständlich? bool funktion muss es bleiben, da vorgabe des profs
camper schrieb:
} template <class T> Stack<T>::~Stack(void) { while(!is_empty()) pop(); } ////////////// ITERATOR ////////////// template <class T> Stack<T>::iterator::iterator(list_el *pos) { // Initialisierungsliste ? pElement = pos; } template <class T> void Stack<T>::iterator::operator++(void) { pElement = pElement->pnext; if(pElement == NULL) throw StackError("iterator hinter Stackende geschaltet!"); } template <class T> void Stack<T>::iterator::operator+=(const int n) { int i; for(i = 0; i >= n; i++) { // du hast bereits op++ geschrieben pElement = pElement->pnext; // warum alles doppelt ? if(pElement == NULL) // ruf doch einfach den anderen operator auf throw StackError("iterator hinter Stackende geschaltet!"); // } } template <class T> void Stack<T>::iterator::erase(void) { // Funktion ? } template <class T> void Stack<T>::iterator::insert(void) { // Funktion ? } template <class T> iterator Stack<T>::begin(void) { // iterator ist in Stack<T> verschachtelter Bezeichner -> Stack<T>::iterator Stack<T>::iterator it(stack_pointer); // stack_pointer zeigt immer auf erstes Element // außerdem ist es ein Typ, der von T abhängt return it; // -> typename Stack<T>::iterator Stack<T>::begin() { ... } template <class T> iterator Stack<T>::end(void) { // dito: typename Stack<T>::iterator Stack<T>::end() { ... for(;;) { if(pElement->pnext == NULL) // hangelt sich bis zum letzten element durch // etwas stimmt hier nicht... Stack<T>::iterator it(pElement); // Stack<T>:: ist hier nicht notwendig, da wir uns ohnehin im Scope der Klasse befinden pElement = pElement->pnext; // zudem musst du keine benannte Variable verwenden return it; // gib den Wert doch direkt zurück: return iterator(pElement) } } #endif // STACK_H_
usw den rest...
-
ex.aveal schrieb:
nicht überflüssig, sonst wird das element auf den der stack_pointer gezeigt hat nicht entfernt
Doch, ist wirklich unnötig hier auf NULL zu setzen, noch hinzu kommt, dass temp als lokale Variable sowieso nach dem Ende des Gültigkeitsbereichs nicht mehr zur Verfügung steht.
MfG,
Probe-Nutzer
-
wie weniger umständlich? bool funktion muss es bleiben, da vorgabe des profs
Was hältst du von
template <class T> bool Stack<T>::is_empty(void) { return stack_pointer == NULL; }
Außerdem ist dein Stack<T>::push falsch. new gibt nicht NULL zurück, sondern wirft eine Exception, wenn kein Speicher mehr verfügbar ist.
Dadurch dass du die nicht fängst ist schonmal das throw (StackError) falsch und dein Programm beendet sich einfach.
-
ja okay, is wohl kürzer, werds mal noch ändern.
aber die push funktion funktioniert zumindestens und das programm beendet sich nicht selbst. leider habe ich exceptions noch nicht wirklich behandelt, daher kann ich nicht nachvollziehen was da jetzt falsch sein sollte etc. muss mich da selber erst einarbeiten
Probe-Nutzer schrieb:
ex.aveal schrieb:
nicht überflüssig, sonst wird das element auf den der stack_pointer gezeigt hat nicht entfernt
Doch, ist wirklich unnötig hier auf NULL zu setzen, noch hinzu kommt, dass temp als lokale Variable sowieso nach dem Ende des Gültigkeitsbereichs nicht mehr zur Verfügung steht.
MfG,
Probe-Nutzer
der erste datensatz bleibt laut meinem prof sonst im speicher und wird erst bei programmende wieder freigegeben. aufdauer würde es sonst eben zum absturz kommen. temp == NULL zu setzen ist nicht zwingend nötig, aber "guter programmier stil" ... wieder laut prof.
aber trotzdem danke nochmal an alle für die hilfe
-
ex.aveal schrieb:
temp == NULL zu setzen ist nicht zwingend nötig, aber "guter programmier stil" ... wieder laut prof.
Dein Prof ist vielleicht ein guter Theoretiker, aber danach hört es auf. Lokale Variablen die nicht mehr verwendet werden (oder auch Membervariablen die nicht mehr verwendet werden) brauchen nicht extra eine Sonderbehandlung.
Besonders schön sind solche unnötigen Codekonstrukte (wo ein "delete zeiger" ausgereicht hätte):
Klassenname::~Klassenname() { if(!zeiger) { delete zeiger; zeiger = 0; } }
So im realen Projekt vorhanden. Und was Programmierstil angeht: Code soll übersichtlich sein, wenn unnötige Zeilen den Code verlängern reduziert dies ebenso die Lesbarkeit wie schlechtes Einrücken.
cu André
-
ex.aveal schrieb:
aber die push funktion funktioniert zumindestens und das programm beendet sich nicht selbst.
Ja, damit sich das Programm beendet, muss auch das allokieren des Speichers fehlschlagen. Vermutlich hast Du diese Situation einfach noch nicht erreicht.
Der Punkt ist aber, dass Dein Programm niemals, also auch nicht wenn die Allokation schiefgeht, einen StackError auslösen kann. Denn diese Form von new liefert niemals NULL als Ergebnis.
-
LordJaxom schrieb:
Denn diese Form von new liefert niemals NULL als Ergebnis.
Doch auf SEHHHR alten Compilern (VC6 und Co.)
Vielleicht sollte sich der Dozent vom OP mal mit den C++ Standard auseinander setzen (Nachdem er nun 10 Jahre draußen ist, und der nächste vor der Tür steht).
cu André
-
und wie würde dann eine lösung des problems aussehen?
wie und was müßte ich denn dann aufrufen?!
-
Das hier sollte so funktionieren, wie du es dir vorgestellt hast:
template <class T> void Stack<T>::push(const T &element) throw (StackError) { try { stack_pointer = new list_el(stack_pointer, element); } catch (std::bad_alloc) { throw StackError("kein Heapspeicher frei!"); } }