Problem with linked list ..

A

Andrew Skouloudis

Hello people and merry christmas. I am trying to create a simple
linked list and it seems i am
doing something wrong with the DeleteNode function ...

#include <malloc.h>
#include <stdio.h>
#include <stdlib.h>
#include <iostream.h>
#include "GenericStringClass2.h"
#include <string.h>


#define DEFAULT 0

class MACListPart
{
public:
// constructor
MACListPart();
~MACListPart();
MACListPart *GetNextPointer();


private:
friend class MACList;
String macAdr;
int data;
MACListPart *next;
};


class MACList
{
public:
//constructors
MACList();
~MACList();

//accessors
void PrintList(void);
void AddNode(const String& mac);
void DeleteNode(const String& mac);
bool FindNode(const String&);
// void DeleteList();
bool IsListEmpty();
int GetCount(void);

protected:
MACListPart *head;
MACListPart *current;

private:
int itsCount;
};

MACListPart::MACListPart(void)
{
next=NULL;
data=0;

// macAdr=..
}


MACListPart::~MACListPart(void)
{
cout << "Calling the default destructor for MACListPart ...
\n";
if(this != NULL)
{

delete this;
}
else delete this;
}

MACListPart* MACListPart::GetNextPointer(void)
{
if(this==NULL)
{
cout << "You are pointing to nowhere , where do you
think you are going ??\n";
return NULL;
}
else
{
return this->next;
}
}


MACList::MACList(void)
{
head=NULL;
current=NULL;
itsCount=0;
}


MACList::~MACList(void)
{/*
if(IsListEmpy()==true)
{
//do nothing ..
}
else
{
DeleteList();
}*/
}

bool MACList::IsListEmpty(void)
{
if(head==NULL)
{
return true;
}

//else ....
return false;
}


bool MACList::FindNode(const String& mac)
{
MACListPart *pMacListPart=head;
while(pMacListPart !=NULL)
{
if(pMacListPart->macAdr==mac)
{
return true;
}
}
return false;
}


int MACList::GetCount(void)
{
return itsCount;
}


void MACList::AddNode(const String& mac)
{
MACListPart *pMACListPart = new MACListPart;
pMACListPart->macAdr=mac;
pMACListPart->data=DEFAULT;
pMACListPart->next=NULL;

if(IsListEmpty() == true)
{
head=pMACListPart;
current=head;
}
else // (if IsListEmpty() == false )
{
current->next=pMACListPart;
current=pMACListPart;
}
// increasing the elements of the list
itsCount++;
}




void MACList::printList()
{
MACListPart *temp = head;
while( temp != NULL)
{
cout << temp->macAdr.GetString() << endl;
temp=temp->next;
}
}


void MACList::DeleteNode(const String& mac)
{
char c;
// If the first element of the list is going to be deleted:
if(head->macAdr==mac)
{
MACListPart *tempNode;
tempNode=head->next;
delete head;
head=tempNode;
}
else
{
MACListPart *prevNode,*currNode;
prevNode=head;
currNode=head->next;
while(currNode != NULL)
{
if(currNode->macAdr==mac)
{
MACListPart *tempNode;
tempNode=currNode->next;
prevNode->next=tempNode;
printf("YAHOOOO\n");
this->PrintList();
c=getchar();
delete currNode;

}
else // you didn't find the string in currNode
so ...
{
prevNode=currNode;
currNode=currNode->next;
}
}
}
}


int main(void)
{
MACList MACListIstance;
MACListIstance.AddNode("11-11-11-11-11-11");
MACListIstance.AddNode("22-22-22-22-22-22");
MACListIstance.AddNode("33-33-33-33-33-33");
MACListIstance.AddNode("44-44-44-44-44-44");
MACListIstance.PrintList();
bool t = MACListIstance.FindNode("11-11-11-11-11-11");
MACListIstance.DeleteNode("22-22-22-22-22-22");
MACListIstance.PrintList();
if (t==true) cout << "This MAC address exists into the
database ... \n";
else cout << "This MAC address does not exist in the
database.... \n";
cout << "Hello world ... the list currently has " <<
MACListIstance.GetCount() << " elements \n";
return 0;
}


The problem is when i am trying to delete a node .. The program hangs
after the command
delete currNode is executed ... The String class is declared in
another file and here is the
default destructor :


String::~String()
{
cout << "Calling the default destructor of String ..\n";
delete [] itsString;
itsLen=0;
cout <<"Default destructor of String finished ... \n";
}


Can you help ?? I am using Visual C++

(Sorry for my english ... )
 
B

Brian MacBride

Andrew Skouloudis said:
Hello people and merry christmas. I am trying to create a simple
linked list and it seems i am
doing something wrong with the DeleteNode function ...


class MACListPart
{
public:
// constructor
MACListPart();
~MACListPart();
MACListPart *GetNextPointer();


private:
friend class MACList;
String macAdr;
int data;
MACListPart *next;
};



MACListPart::~MACListPart(void)
{
cout << "Calling the default destructor for MACListPart ...
\n";
/*

if(this != NULL)
{

delete this;
}
else delete this;
*/

}

You don't need to write destructor for MACListPart. If you do... you don't
want to do the suicide thing...

Remember to decrement itsCount in the

void MACList::DeleteNode (const String &mac) function and you'll get...

11-11-11-11-11-11
22-22-22-22-22-22
33-33-33-33-33-33
44-44-44-44-44-44
YAHOOOO
11-11-11-11-11-11
33-33-33-33-33-33
44-44-44-44-44-44
11-11-11-11-11-11
33-33-33-33-33-33
44-44-44-44-44-44
This MAC address exists into the database ...
Hello world ... the list currently has 3 elements

Is that what you wanted?
Can you help ?? I am using Visual C++

(Sorry for my english ... )

Regards

Brian
 
A

Andrew Skouloudis

You don't need to write destructor for MACListPart. If you do... you don't
want to do the suicide thing...

Remember to decrement itsCount in the

void MACList::DeleteNode (const String &mac) function and you'll get...

11-11-11-11-11-11
22-22-22-22-22-22
33-33-33-33-33-33
44-44-44-44-44-44
YAHOOOO
11-11-11-11-11-11
33-33-33-33-33-33
44-44-44-44-44-44
11-11-11-11-11-11
33-33-33-33-33-33
44-44-44-44-44-44
This MAC address exists into the database ...
Hello world ... the list currently has 3 elements

Is that what you wanted?


Regards

Brian
First of all thank you for your answer
Secondly .. how did you do that ??

I did put

if(this != NULL)
{

delete this;
}
else delete this;

in comments and still the program hangs !! Can you give me the
destructor of your String class ????
I even removed the destructor definition in the MACListPart , so the
compiler should do whatever it want to, when deleting the node .
But still nothing !!! .

This is the output of my program :

Calling the default destructor of String ..
Default destructor of called ...
Calling the default destructor of String ..
Default destructor of called ...
Calling the default destructor of String ..
Default destructor of called ...
Calling the default destructor of String ..
Default destructor of called ...
11-11-11-11-11-11
22-22-22-22-22-22
33-33-33-33-33-33
44-44-44-44-44-44
YAHOOOO
Calling the default destructor of String ..
Default destructor of called ...
11-11-11-11-11-11
33-33-33-33-33-33
44-44-44-44-44-44 // And now hangs !!!
 
B

Brian MacBride

Andrew Skouloudis said:
First of all thank you for your answer
Secondly .. how did you do that ??

I did put

if(this != NULL)
{

delete this;
}
else delete this;

in comments and still the program hangs !! Can you give me the
destructor of your String class ????
I even removed the destructor definition in the MACListPart , so the
compiler should do whatever it want to, when deleting the node .
But still nothing !!! .

This is the output of my program :

Calling the default destructor of String ..
Default destructor of called ...
Calling the default destructor of String ..
Default destructor of called ...
Calling the default destructor of String ..
Default destructor of called ...
Calling the default destructor of String ..
Default destructor of called ...
11-11-11-11-11-11
22-22-22-22-22-22
33-33-33-33-33-33
44-44-44-44-44-44
YAHOOOO
Calling the default destructor of String ..
Default destructor of called ...
11-11-11-11-11-11
33-33-33-33-33-33
44-44-44-44-44-44 // And now hangs !!!

You didn't forget that you have this silly....

c = getchar ();

....here. Lose it...

Regards

Brian
 
A

Andrew Skouloudis

You didn't forget that you have this silly....

c = getchar ();

...here. Lose it...

Regards

Brian

I am not THAT stupid ... When I i say it hangs i mean that the
execution of the program stops and Windows XP creates a window
which says that " LinkedList3.exe has encourted a problem and needs
to close . We are sorry for the inconvenience " blah-blah .. Debug ,
Send Report , Don't Send ". The problem is with the delete operation .
Did you run this program with VC++ ??? . What's your String destructor
???
 
M

Michael Mellor

Andrew said:
Hello people and merry christmas. I am trying to create a simple
linked list and it seems i am
doing something wrong with the DeleteNode function ...

#include <malloc.h>
#include <stdio.h>
#include <stdlib.h>
#include <iostream.h>
iostream.h is not standard so use:
#include <iostream>
and for the time being
using namespace std;
#include "GenericStringClass2.h"
#include <string.h>


#define DEFAULT 0

class MACListPart
{
public:
// constructor
MACListPart();
~MACListPart();
MACListPart *GetNextPointer();


private:
friend class MACList;
String macAdr;
int data;
MACListPart *next;
};


class MACList
{
public:
//constructors
MACList();
~MACList();

//accessors
void PrintList(void);
void AddNode(const String& mac);
void DeleteNode(const String& mac);
bool FindNode(const String&);
// void DeleteList();
bool IsListEmpty();
int GetCount(void);

protected:
MACListPart *head;
MACListPart *current;

private:
int itsCount;
};

MACListPart::MACListPart(void)
{
next=NULL;
data=0;

// macAdr=..
}


MACListPart::~MACListPart(void)
{
cout << "Calling the default destructor for MACListPart ...
\n";
if(this != NULL)
{

delete this;
}
else delete this;
This is quite a strange piece of code as it is equivalent to "delete
this". It is not needed.
}

MACListPart* MACListPart::GetNextPointer(void)
{
if(this==NULL)

If this is NULL you have problems, therefore this check is completely
unneeded. Do the NULL check where you use this method.
{
cout << "You are pointing to nowhere , where do you
think you are going ??\n";
return NULL;
}
else
{
return this->next;
}
}


MACList::MACList(void)
{
head=NULL;
current=NULL;
itsCount=0;
}


MACList::~MACList(void)
{/*
if(IsListEmpy()==true)
{
//do nothing ..
}
else
{
DeleteList();
}*/
}

bool MACList::IsListEmpty(void)
{
if(head==NULL)
{
return true;
}

//else ....
return false;
}


bool MACList::FindNode(const String& mac)
{
MACListPart *pMacListPart=head;
while(pMacListPart !=NULL)
{
if(pMacListPart->macAdr==mac)
{
return true;
}
}
return false;
}


int MACList::GetCount(void)
{
return itsCount;
}


void MACList::AddNode(const String& mac)
{
MACListPart *pMACListPart = new MACListPart;
pMACListPart->macAdr=mac;
pMACListPart->data=DEFAULT;
pMACListPart->next=NULL;

if(IsListEmpty() == true)
{
head=pMACListPart;
current=head;
}
else // (if IsListEmpty() == false )
{
current->next=pMACListPart;
current=pMACListPart;
}
// increasing the elements of the list
itsCount++;
}




void MACList::printList()
{
MACListPart *temp = head;
while( temp != NULL)
{
cout << temp->macAdr.GetString() << endl;
temp=temp->next;
}
}


void MACList::DeleteNode(const String& mac)
{
char c;
// If the first element of the list is going to be deleted:
if(head->macAdr==mac)
{
MACListPart *tempNode;
tempNode=head->next;
delete head;
head=tempNode;
}
else
{
MACListPart *prevNode,*currNode;
prevNode=head;
currNode=head->next;
while(currNode != NULL)
{
if(currNode->macAdr==mac)
{
MACListPart *tempNode;
tempNode=currNode->next;
prevNode->next=tempNode;
printf("YAHOOOO\n");
this->PrintList();
c=getchar();
delete currNode;
The reason it crashes is because you dont exit the loop. Just add a
break; here. Or if you want to delete all nodes that match update
prevNode and currNode.

Mike
 
A

Andrew Skouloudis

iostream.h is not standard so use:
#include <iostream>
and for the time being
using namespace std;

This is quite a strange piece of code as it is equivalent to "delete
this". It is not needed.

If this is NULL you have problems, therefore this check is completely
unneeded. Do the NULL check where you use this method.

The reason it crashes is because you dont exit the loop. Just add a
break; here. Or if you want to delete all nodes that match update
prevNode and currNode.

Mike

Yes that's it .. THANK YOU SO MUCH !!!!!!!!!!!!!!!
 
B

Brian MacBride

Andrew Skouloudis said:
I am not THAT stupid ...

I didn't mean to imply that you were. I often overlook the obvious at
times. Sometimes I can't see the forest for the trees. ;-)
When I i say it hangs i mean that the
execution of the program stops and Windows XP creates a window
which says that " LinkedList3.exe has encourted a problem and needs
to close . We are sorry for the inconvenience " blah-blah .. Debug ,
Send Report , Don't Send ". The problem is with the delete operation .

One of the things that would give me that message is running out of stack
space.

This...

MACListPart::~MACListPart (void) {
cout << "Calling the default destructor for MACListPart ...\n";
if (this != NULL)
delete this;
else
delete this;
}

.... will set up an endless loop and would have caused that.that message for
me.
Did you run this program with VC++ ???.

....and two others... g++ amd Borland
What's your String destructor
???

Well I don't have your String class but your d'tor looks OK and would have
worked for you elsewhere in your program.

I used std::string, implemented thus...

#include <malloc.h>
#include <stdio.h>
#include <stdlib.h>
#include <iostream.h>
// #include "GenericStringClass2.h"
#include <string.h>

// Emulate 'class String'
#include <string>
typedef std::string String;
#define GetString c_str

#define DEFAULT 0

<snip>

,,,You could try that.

Regards

Brian
 
B

Brian MacBride

Brian MacBride said:
I didn't mean to imply that you were. I often overlook the obvious at
times. Sometimes I can't see the forest for the trees. ;-)

See, I told you that we all overlook the obvious at times. As Michael Mellor
did, I
added a break to the loop in your void MACList::DeleteNode (const String
&mac) function, and then forgot to mention that to you. Sorry.

Regards

Brian
 
W

Wagner Bruna

Hi,

Andrew Skouloudis said:
Hello people and merry christmas. I am trying to create a simple
linked list and it seems i am
doing something wrong with the DeleteNode function ...

#include <malloc.h>
#include <stdio.h>
#include <stdlib.h>
#include <iostream.h>
#include "GenericStringClass2.h"
#include <string.h>

Many of these headers are not necessary in your program. Also, you
#define DEFAULT 0

"const int DEFAULT = 0;" is better here.

(...)

MACListPart::~MACListPart(void)
{
cout << "Calling the default destructor for MACListPart ...
\n";
if(this != NULL)
{

delete this;
}
else delete this;

As someone else has pointed out, you shouldn't do this. See the FAQ
for details:

http://www.parashift.com/c++-faq-lite/

}

MACListPart* MACListPart::GetNextPointer(void)
{
if(this==NULL)
{
cout << "You are pointing to nowhere , where do you
think you are going ??\n";
return NULL;
}
else
{
return this->next;
}

Here, IMHO, "assert(this != NULL)" would make more sense...

(...)
bool MACList::FindNode(const String& mac)
{
MACListPart *pMacListPart=head;
while(pMacListPart !=NULL)
{
if(pMacListPart->macAdr==mac)
{
return true;
}
}
return false;
}

You forgot to update pMacListPart.

(...)
void MACList::AddNode(const String& mac)
{
MACListPart *pMACListPart = new MACListPart;
pMACListPart->macAdr=mac;
pMACListPart->data=DEFAULT;
pMACListPart->next=NULL;

if(IsListEmpty() == true)
{
head=pMACListPart;
current=head;
}
else // (if IsListEmpty() == false )
{
current->next=pMACListPart;
current=pMACListPart;

If "current" is used only to help insertions at the end of the list,
its name is misleading; also, you may need to update it when you
delete a node.

}
// increasing the elements of the list
itsCount++;
}

(...)

The problem is when i am trying to delete a node .. The program hangs
after the command
delete currNode is executed ... The String class is declared in
another file and here is the
default destructor :

Hard to say anything without the complete String class, but you could
for example replace it with std::string (or, better yet, int) for
testing MACList separately.

HTH,
Wagner
 
A

Andrew Skouloudis

See, I told you that we all overlook the obvious at times. As Michael Mellor
did, I
added a break to the loop in your void MACList::DeleteNode (const String
&mac) function, and then forgot to mention that to you. Sorry.

Regards

Brian

I would like to thank you for your help ... I am new to C++
programming so I make many mistakes !!!
 
A

Andrew Skouloudis

Hi,



Many of these headers are not necessary in your program. Also, you


"const int DEFAULT = 0;" is better here.



As someone else has pointed out, you shouldn't do this. See the FAQ
for details:

http://www.parashift.com/c++-faq-lite/



Here, IMHO, "assert(this != NULL)" would make more sense...



You forgot to update pMacListPart.



If "current" is used only to help insertions at the end of the list,
its name is misleading; also, you may need to update it when you
delete a node.



Hard to say anything without the complete String class, but you could
for example replace it with std::string (or, better yet, int) for
testing MACList separately.

HTH,
Wagner


Thank you very much !! . Do you know whre I can download a zip file
containing the entire faq ??? (The administrator of the web site
http://www.parashift.com/c++-faq-lite/ has disabled that option !!)
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,755
Messages
2,569,537
Members
45,020
Latest member
GenesisGai

Latest Threads

Top