005
10.07.2008, 01:14 Uhr
0xdeadbeef
Gott
(Operator)
|
Okay, mal ernsthaft zum Code - kein schlechter Anfang, aber ich hab doch ein paar grundsätzliche Bemerkungen dazu. Den Kleinkram zuerst:
ist im Standard nicht enthalten. Der übliche Weg hier ist, Include-Guards zu benutzen:
| C++: |
#ifndef INCLUDED_HEADERNAME_HIER
#define INCLUDED_HEADERNAME_HIER
// Eigentlicher Code hier
#endif
|
Damit kannst du dir sicher sein, dass der Code auch mit anderen Compilern kompiliert. Naja, oder zumindest, dass es nicht an einem unbekannten Pragma scheitert.
Jetzt zur echten Substanz:
Erstens fällt mir auf, dass du zwar AbstractDoubleLinkedList::add(int) pure virtual deklarierst (=0), aber trotzdem implementierst. Das ist mindestens schlechter Stil, und mit einiger Wahrscheinlichkeit illegal.
Zweitens ist eine Liste ohne echte Zugriffsfunktion keine wirkliche Liste. Dazu kommt, dass deine einzige Zugriffsfunktion auf das head-Element das head-Element auch noch entfernt - damit sind wir vom Konzept einer Liste schon seeehr weit entfernt.
Überhaupt stellt sich mir die Frage, warum die Liste abstrakt sein soll. Bau doch einfach eine doppelt verkettete Liste, und lass deinen Stack bzw. deine Queue die einfach benutzen. Einfache Prototypisierung:
| C++: |
class List {
public:
List();
void insert(std::size_t pos, int wert);
void remove(std::size_t pos);
int get(std::size_t pos) const;
std::size_t size() const;
};
class Stack { // FIFO
public:
Stack();
void push(int wert);
int pop();
private:
List ls;
};
class Queue { // LIFO
public:
Queue();
void enqueue(int wert);
int dequeue();
private:
List ls;
};
|
Ansonsten würde ich die Node-Klasse wahrscheinlich von ihren Methoden befreien und ein einfaches Struct a la
| C++: |
struct Node {
Node *prev, *next;
int wert;
};
|
benutzen, und die Verkettungslogik in die Liste verlegen - aber das ist ein bisschen Geschmackssache. Meiner Erfahrung nach ist es hier übersichtlicher, die ganze Logik in der Liste zu erledigen, weil die Nebeneffekte der Node-Methoden nicht immer sofort ersichtlich sind. Zum Beispiel wird beim Löschen eines Elements etwas in der Art auftreten:
| C++: |
Node *n;
n = zu_loeschende_node;
n->getPrevious()->setNextNode(n->getNextNode());
delete n;
|
...und aus der langen Zeile da zu sehen, dass n nachher befreit und n->getNextNode()->getPrevious() auch n->getPrevious() ist, ist für mein Verständnis weniger intuitiv als wenn da einfach
| C++: |
n->prev->next = n->next;
n->next->prev = n->prev;
delete n;
|
stünde. Aber wie gesagt, das ist ein bisschen Geschmackssache.
--
Einfachheit ist Voraussetzung für Zuverlässigkeit.
-- Edsger Wybe Dijkstra |
find das rar schon okay.
