short linked list

[bejiz]

Hello,
I have written a short program for practising linked lists. But there
is surely something wrong for when I compile there is a unhandled
exception and it does not print if I try to add more than one element
to the list. Is this a problem with the function of the internal class
Node ?
Thanks for any help.
Here is the code:

#include<iostre am>
using namespace std;

class UseNodes
{
private:
class Node
{
public:
Node()
{
Data = 1;
Next = 0;
}
Node(int a):Data(a){}
~Node(){};
Node* GetNext()const{ return Next;}
void SetNext( Node* A){ Next = A;}
int GetData()const{ return Data;}
void setData(int a) { Data = a;}
private:
Node* Next;
int Data;
};
Node* Head;
Node* Tail;

public:
UseNodes(){Head = 0; Tail = 0; }
~UseNodes(){};
void AddNode(int a)
{
Node* Temp = new Node(a);
if(Head==0)
{
Head = Temp;
Tail = Temp;
}
else
{
Node* Current = new Node();
Current = Head;
while(Current)
{
Current = Current->GetNext();
}
Current->SetNext(Temp );
Tail = Temp;
}
}
void Print()const
{
int a = 2;
Node* Temp = new Node(a);
if(Head==0)
cout << " Empty list.\n " ;
else
{
Temp = Head;
cout << Head->GetData() << endl;
while(Temp!=Tai l->GetNext())
{
cout << Temp->GetData() << endl;
Temp = Temp->GetNext() ;
}
}
}

};

int main()
{
UseNodes A;
A.AddNode(29);
A.AddNode(7777) ;
A.Print();

getchar();
return 0;
}

[Ivan Vecerina]

Re: short linked list

"bejiz" <bruno.julier@w anadoo.frwrote in message
news:1161095190 .303145.217130@ h48g2000cwc.goo glegroups.com.. .
: Hello,
: I have written a short program for practising linked lists. But there
: is surely something wrong for when I compile there is a unhandled
: exception and it does not print if I try to add more than one element
: to the list. Is this a problem with the function of the internal class
: Node ?
Just quickly skimming through the code...

: Thanks for any help.
: Here is the code:
:
: #include<iostre am>
: using namespace std;
:
: class UseNodes
: {
: private:
: class Node
: {
: public:
: Node()
: {
: Data = 1;
: Next = 0;
: }
NB: initialization lists usually should be preferred.
Node() : Data(1), Next(0) {}
Note that it is probably best to omit this default
constructor.

: Node(int a):Data(a){}
This constructor leaves Next uninitialized.
This is probably the cause of the crash.

Also, for technical reasons (read into it), you would
want to label this constructor as explicit:
explicit Node(int a)
: Data(a), Next(00)
{ }

: ~Node(){};
: Node* GetNext()const{ return Next;}
: void SetNext( Node* A){ Next = A;}
: int GetData()const{ return Data;}
: void setData(int a) { Data = a;}
You're mixing upper and lowercase names... ('s'etData).
I would recommend using:
- leading uppercase letter for Class and Type names only
- a trailing underscore for data members

: private:
: Node* Next;
: int Data;
: };
: Node* Head;
: Node* Tail;
:
: public:
: UseNodes(){Head = 0; Tail = 0; }
: ~UseNodes(){};
: void AddNode(int a)
: {
: Node* Temp = new Node(a);
: if(Head==0)
: {
: Head = Temp;
: Tail = Temp;
: }
: else
: {
: Node* Current = new Node();
Should be: new Node(a);

: Current = Head;
: while(Current)
: {
: Current = Current->GetNext();
: }
: Current->SetNext(Temp );
Couldn't you use Tail instead of going through
the whole list to find the last element ?

: Tail = Temp;
: }
: }
: void Print()const
: {
: int a = 2;
: Node* Temp = new Node(a);
What is this for ?
Just remove the two previous lines...

: if(Head==0)
: cout << " Empty list.\n " ;
: else
: {
: Temp = Head;
Best is to declare variables upon first use.
That is, here, replace the previous line with:
Note* temp = head;

: cout << Head->GetData() << endl;
: while(Temp!=Tai l->GetNext())
: {
: cout << Temp->GetData() << endl;
: Temp = Temp->GetNext() ;
: }
Very confused, and incorrect again.
You could write:
do {
cout << Temp->GetData() << endl;
Temp = Temp->GetNext();
} while( Temp != 00 );

: }
: }
:
: };
:
: int main()
: {
: UseNodes A;
: A.AddNode(29);
: A.AddNode(7777) ;
: A.Print();
:
: getchar();
: return 0;
: }


hth -Ivan
--
http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact form
Brainbench MVP for C++ <http://www.brainbench.com

[bejiz]

Re: short linked list



Thanks for your detailed advices , my linked list is now working . I
didn't know how to define "Next" , and the functions "AddNode" and
"Print" were quite entangled .
Bejiz

[Salt_Peter]

Re: short linked list


bejiz wrote:

Hello,
I have written a short program for practising linked lists. But there
is surely something wrong for when I compile there is a unhandled
exception and it does not print if I try to add more than one element
to the list. Is this a problem with the function of the internal class
Node ?
Thanks for any help.
Here is the code:
>
#include<iostre am>
using namespace std;
>
class UseNodes
{
private:
class Node
{
public:
Node()
{
Data = 1;
Next = 0;
}
Node(int a):Data(a){}
~Node(){};
Node* GetNext()const{ return Next;}
void SetNext( Node* A){ Next = A;}
int GetData()const{ return Data;}
void setData(int a) { Data = a;}
private:
Node* Next;
int Data;
};

Your Node class is already a private embedded member of the List class.
There is no need to provide accessors (set/get functions). A ctor
alone will do with init list. Always, always intialize all your
members. Specially pointers like Next above.

Node* Head;
Node* Tail;
>
public:
UseNodes(){Head = 0; Tail = 0; }
~UseNodes(){};
void AddNode(int a)
{
Node* Temp = new Node(a);
if(Head==0)
{
Head = Temp;
Tail = Temp;
}
else
{
Node* Current = new Node();
Current = Head;
while(Current)
{
Current = Current->GetNext();
}
Current->SetNext(Temp );
Tail = Temp;
}
}
void Print()const
{
int a = 2;
Node* Temp = new Node(a);
if(Head==0)
cout << " Empty list.\n " ;
else
{
Temp = Head;
cout << Head->GetData() << endl;
while(Temp!=Tai l->GetNext())
{
cout << Temp->GetData() << endl;
Temp = Temp->GetNext() ;
}
}
}
>
};
>
int main()
{
UseNodes A;
A.AddNode(29);
A.AddNode(7777) ;
A.Print();
>
getchar();
return 0;
}

A proper naming convention will help your code. Your class represents a
single linked list - then call it that: SingleLinkedLis t or something
relevant. That way, code like this becomes reusable. One day you'll
scan your projects and something like "UseNodes" won't describe a
single linked list.
Also, never use new without a corresponding delete.

here is one that will store anything, templates are much more usefull
than they are complex.


#include <iostream>
#include <ostream>
#include <string>

template< typename T >
class SingleLinkedLis t
{
struct Node
{
T t;
Node* p_next;
Node(T val, Node* p = 0) : t(val), p_next(p) { }
} *p_head, *p_tail;
public:
SingleLinkedLis t() : p_head(0), p_tail(0) { }
SingleLinkedLis t(const SingleLinkedLis t& r_l); // disabled copy ctor
~SingleLinkedLi st() { clear(); }
/* member functions */
void push_back(const T& t)
{
if (p_head == 0)
{
p_head = p_tail = new Node(t);
} else {
p_tail->p_next = new Node(t);
p_tail = p_tail->p_next;
}
}
void clear()
{
while(p_head != 0)
{
Node* p_zap = p_head;
p_head = p_head->p_next;
delete p_zap;
}
p_tail = 0;
}
/* friends */
friend
std::ostream& operator<<(std: :ostream& os, const SingleLinkedLis t&
r_l)
{
Node* piterator(r_l.p _head);
while(piterator != 0)
{
os << piterator->t << "\n";
piterator = piterator->p_next;
}
return os;
}
};

int main()
{
SingleLinkedLis t< int nlist;
for(int n = 0; n < 5; ++n)
{
nlist.push_back (n);
}
std::cout << nlist;

SingleLinkedLis t< double dlist;
for(int i = 0; i < 5; ++i)
{
dlist.push_back (static_cast<do uble>(i) / 4);
}
std::cout << dlist;

SingleLinkedLis t< std::string slist;
slist.push_back ("string_0") ;
slist.push_back ("string_1") ;
slist.push_back ("string_2") ;
std::cout << slist;

return 0;
}

/*
0 <- nlist
1
2
3
4
0 <- dlist
0.25
0.5
0.75
1
string_0 <- slist
string_1
string_2
*/

[Thomas J. Gritzan]

Re: short linked list

Ivan Vecerina schrieb:

"bejiz" <bruno.julier@w anadoo.frwrote in message

[...]

: void AddNode(int a)
: {
: Node* Temp = new Node(a);
: if(Head==0)
: {
: Head = Temp;
: Tail = Temp;
: }
: else
: {
: Node* Current = new Node();
Should be: new Node(a);
>
: Current = Head;

No, should be:

Node* Current = Head;

: while(Current)
: {
: Current = Current->GetNext();
: }
: Current->SetNext(Temp );

At this point, Current is NULL. You can't use it. Better:

while (Current->GetNext())
{
Current = Current->GetNext();
}

Or better use Tail as Ivan suggested.

Couldn't you use Tail instead of going through
the whole list to find the last element ?
>
: Tail = Temp;
: }

--
Thomas

http://www.netmeister.org/news/learn2quote.html