Problem using delete (Linked Lists)

[Jonas Ferreira]

Hi everyone, I'm trying to code a linked list example and everything
was working fine. Then I started to code my clear() (to clear my list)
but it won't work. Here is the code:

class list {
protected:
struct node {
char* data;
node* prev;
node* post;
node(): prev(NULL),post(NULL) {};
node(char* u): data(u),prev(NULL),post(NULL) {};
};
node* head;
node* tail;
int QS,qtd;
public:
list(int size,char* u);
void go(char* u);
void show();
void clear();
};

void list::clear()
{
node *L,*D;
int I;
for (L=head;L!=tail;L=D) { D=L->post; delete L; };
cout << "clear(): " <<endl;
};

I didn't post all the code here but I can send the rest of it if anyone
needs (wishes).
Well, I can compile it and even run it, but "clear()" won't work.
I wrote a main.cpp just to populate the list, then show its nodes,
clear the list, then show its nodes again (it should be 0 nodes, after
the clear()), but after the clear, its nodes are the very same before
the clear.

---main.cpp----
#include stuff
int main() {
list L(20,"teste");
L.go("papa01");
L.go("papa02");
L.go("papa03");
L.go("papa04");
L.show();
L.clear();
L.show();
};
----------------

the output is:
----
papa01
papa02
papa03
papa04
clear():
papa01
papa02
papa03
papa04
-----

If anyone can help just saying what is wrong, I would appreciate it.
Thanks, Jonas.

ps: Sorry about the poor english.

 

[Alf P. Steinbach]

* Jonas Ferreira:

Hi everyone, I'm trying to code a linked list example and everything
was working fine. Then I started to code my clear() (to clear my list)
but it won't work. Here is the code:

class list {
protected:
struct node {
char* data;
node* prev;
node* post;
node(): prev(NULL),post(NULL) {};
node(char* u): data(u),prev(NULL),post(NULL) {};
};
node* head;
node* tail;
int QS,qtd;
public:
list(int size,char* u);
void go(char* u);
void show();
void clear();
};

void list::clear()
{
node *L,*D;
int I;
for (L=head;L!=tail;L=D) { D=L->post; delete L; };
cout << "clear(): " <<endl;
};

Avoid non-idiomatic single char names. Don't use all uppercase
identifiers except for macros. Remember to delete the tail node and
null out 'prev' and 'post'.

I didn't post all the code here but I can send the rest of it if anyone
needs (wishes).

No need.

Well, I can compile it and even run it, but "clear()" won't work.
I wrote a main.cpp just to populate the list, then show its nodes,
clear the list, then show its nodes again (it should be 0 nodes, after
the clear()), but after the clear, its nodes are the very same before
the clear.

That's just by happenchance: the memory for each deleted node has been
made available for reuse, but (with this C++ implementation) the
contents haven't been changed.

 

[Alf P. Steinbach]

* Alf P. Steinbach:

null out 'prev' and 'post'.

'head' and 'tail', I meant.

Those names are confusing.

 

[Jonas Ferreira]

Sorry about the names, i had to change them to post here (they were all
in portuguese).
I setted the head and tail pointers to NULL and now everything is
working fine.

You said "with this C++ implementation" the contents remain unchanged.
Is there a way to "nullify" these content?? Is it really necessary?
Because I was taught that you don't need a destructor, because most
compilers already give you one that destroys the builtin types (does
this apply to char* type?).

Really, thanks for the help!

Alf P. Steinbach escreveu:

 

[Grizlyk]

Jonas Ferreira wrote:

1.

node(): prev(NULL),post(NULL) {};
node(char* u): data(u),prev(NULL),post(NULL) {};

much better do like this:

node(char* u=0): data(u),prev(NULL),post(NULL) {};

2.
List must have ctors and dtor

public:
list()throw():head(0),tail(0),QS(?),qtd(?){}
~list()throw(){clear();}

//at least disable copy&assign
private:
list(const list&)throw():head(0),tail(0),QS(?),qtd(?){exit(1);}
void operator= (const list&)throw(){exit(1);}

3.

void list::clear()
{
node *L,*D;
int I;
for (L=head;L!=tail;L=D) { D=L->post; delete L; };
cout << "clear(): " <<endl;
};

Do not use uninitialized vars (*L,*D; I).
What the reason of D?

Learn C/C++ function structure. It is something like this:

chech incoming (bounds etc)
do actions
chech result

After function have returned, class must remain in defined correct
state.
What do with list after clear()?

Learn RAII resource control idiom.

http://www.hackcraft.net/raii

This is nothing wrong with naked pointers using, but It is often no
reasons to do it in new classes, because RAII wrapper often gives
better and clear structure of classes.

To use RAII you need define some types of "holders" with different
behaviour, (something like auto_ptr<>), keeping each naked resource
separatedly.

Note (if you are using unwrapped resources) that the C++ does not
allow automatic call of destructor if ctor throw.