need help with link list

[berkay]

i have a txt file;
berkay#white
jack#black
smith#jane

writes in it.
and after i run the program it only prints smith jane and crashes
what is wrong?


#include<iostream>
#include<fstream>
using namespace std;

class data{
private:
char name[20];
char surname[30];
data *next;
public:
data(){next=NULL;}
friend class list;
void display();
};

class list{
private:
data *head,*tail;
public:
list(){
head=tail=NULL;}
void displayall();
void append(data *);
void load();
};


void data::display()
{
cout<<name<<" "<<surname<<endl;
}


void list::displayall()
{

data *py;
py=head;

if(py==NULL)
{
cout<<"nothing to show"<<endl;
}
while(py!=NULL){//while py points sth reasonable

py->display();

py=py->next;
py->next=NULL;
}

}

void list::append(data *ptr){

if(head==NULL)//first element
{
head=ptr;
tail=ptr;
}
else
{

tail->next=ptr;
tail=tail->next;
tail->next=NULL;

}

}


void list::load(){

ifstream file("dosya.txt",ios::in);

char buf[50];

data *datas;

datas=new data;
while(!file.eof())
{

file.getline(buf,20,'#');

strcpy(datas->name,buf);
file.getline(buf,30,'\n');
strcpy(datas->surname,buf);

append(datas);
}
file.close();

}




void main()
{
list liste;
liste.load();
liste.displayall();


}

 

[Branimir Maksimovic]

i have a txt file;
berkay#white
jack#black
smith#jane

writes in it.
and after i run the program it only prints smith jane and crashes
what is wrong?


void list::displayall()
{

data *py;
py=head;

if(py==NULL)
{
cout<<"nothing to show"<<endl;
}
while(py!=NULL){//while py points sth reasonable

py->display();

py=py->next;
py->next=NULL;

drop this line, in next loop your py->next would be NULL
and then you dererence NULL

}

}

Greetings, Bane.

 

[Kai-Uwe Bux]

i have a txt file;
berkay#white
jack#black
smith#jane

writes in it.
and after i run the program it only prints smith jane and crashes
what is wrong?

The main mistake is that you want to do a list yourself. There is no need
for that:

#include <iostream>
#include <fstream>
#include <list>
#include <algorithm>
#include <string>
#include <iterator>

struct data {
std::string name;
std::string surname;
};

std:stream & operator<< ( std:stream & ostr,
data const & d ) {
ostr << d.name << " " << d.surname;
return( ostr );
}

std::istream & read_wicked_data ( std::istream & istr,
data & d ) {
std::string n;
std::string sn;
if ( std::getline( istr, n, '#' )
&&
std::getline( istr, sn, '\n' ) ) {
d.name = n;
d.surname = sn;
}
return( istr );
}


typedef std::list< data > data_list;

void display_all ( data_list const & dl ) {
std::copy( dl.begin(), dl.end(),
std:stream_iterator<data>( std::cout, "\n" ) );
}

data_list read_file ( std::string const & file_name ) {
data_list result;
std::ifstream file( file_name.c_str(), std::ios::in );
data d;
while ( read_wicked_data( file, d ) ) {
result.push_back( d );
}
return ( result );
}

int main ( void ) {
data_list liste = read_file( "file.txt" );
display_all( liste );
}

#include<iostream>
#include<fstream>
using namespace std;

class data{
private:
char name[20];
char surname[30];
data *next;
public:
data(){next=NULL;}
friend class list;
void display();
};

class list{
private:
data *head,*tail;
public:
list(){
head=tail=NULL;}
void displayall();
void append(data *);
void load();
};


void data::display()
{
cout<<name<<" "<<surname<<endl;
}


void list::displayall()
{

data *py;
py=head;

if(py==NULL)
{
cout<<"nothing to show"<<endl;
}
while(py!=NULL){//while py points sth reasonable

py->display();

py=py->next;
py->next=NULL;
}

}

void list::append(data *ptr){

This interface is a BadIdea (tm): you will need to transfer ownership of the
pointer to the list.

if(head==NULL)//first element
{
head=ptr;
tail=ptr;
}
else
{

tail->next=ptr;
tail=tail->next;
tail->next=NULL;

}

}


void list::load(){

ifstream file("dosya.txt",ios::in);

char buf[50];

data *datas;

datas=new data;

Here you are allocating datas outside the loop.

while(!file.eof())
{

file.getline(buf,20,'#');

strcpy(datas->name,buf);
file.getline(buf,30,'\n');
strcpy(datas->surname,buf);

append(datas);

Here you append datas to the list. This will append the same pointer each
time. This is bad since the list already took ownership of that pointer in
the first iteration.

}
file.close();

}




void main()
{
list liste;
liste.load();
liste.displayall();


}

Best

Kai-Uwe Bux

 

[berkay]

i know i dont need to use link list but am a student and trying to
learn.
while(py!=NULL){//while py points sth reasonable

py->display();

py=py->next;
py->next=NULL;
}
yes here i print py first.it was first equal to head and by using next
pointer i am trying to print the items in the list till the tail so i
think it must bu true.

 

[guyarad]

Why are doing this "py->next=NULL"???
this function should _Display" the elements in the list, not alter
them!!!

think about it:
The first iteration is fine.
after the 1st one, py is pointing to the 2nd element, but for some
reason, py->next is pointing to NULL instead of the 3rd element.
and then you do: py=py->next
so far no illegal access to memory are made.
but now py is NULL, and then you are trying to access it. bas things
happen...

 

[Old Wolf]

i have a txt file;
berkay#white
jack#black
smith#jane

writes in it.
and after i run the program it only prints smith jane and crashes
what is wrong?

You have a lot of errors. The main one is that you are using
the same data node for all of the list elements (ie. every
list element points to the same piece of data). When you read
lines from your file you are overwriting the data from the
previous lines.

The next biggest error is that your list traversing function
actually traverses the first node and then deletes the rest
of the list.

The crashing is because you read past the end of the file and
then try to assign garbage to the list, causing a buffer overflow.

#include<iostream>
#include<fstream>
using namespace std;

class data{
private:
char name[20];
char surname[30];
data *next;
public:
data(){next=NULL;}
friend class list;
void display();
};

You would be better to use std::string for "name" and "surname",
so that you do not cause buffer overflows.

Note that since you have not defined your own copy-constructor,
any copies of a "data" object will point to the same "next"
object.

class list{
private:
data *head,*tail;
public:
list(){
head=tail=NULL;}
void displayall();
void append(data *);
void load();
};

This is very bad indenting style, it makes your code hard to
read. The functions should all be indented to the same level,
and any function bodies should be further indented.

Again you have not got a copy constructor. So this list may
behave erratically if you make copies of it.

Also you have no destructor, so memory will not be freed
when the list is destroyed.

void data::display()
{
cout<<name<<" "<<surname<<endl;
}

void list::displayall()
{
data *py;
py=head;

if(py==NULL)
{
cout<<"nothing to show"<<endl;
}
while(py!=NULL){//while py points sth reasonable

py->display();

py=py->next;
py->next=NULL;

After displaying the first node, you set the second node's
"next" pointer to NULL. So if the list had more than 2 items,
the rest all just got lost.

}
}

void list::append(data *ptr){

if(head==NULL)//first element
{
head=ptr;
tail=ptr;
}
else
{
tail->next=ptr;
tail=tail->next;
tail->next=NULL;
}

}


void list::load(){

ifstream file("dosya.txt",ios::in);

char buf[50];

data *datas;

datas=new data;
while(!file.eof())
{

Bad. eof() tells you if you have already tried to read past the end
and failed. Instead, you should check the file read for success or
failure. This means that your list will get a garbage entry on the end.

file.getline(buf,20,'#');

strcpy(datas->name,buf);
file.getline(buf,30,'\n');
strcpy(datas->surname,buf);

You will get a buffer overflow if the text file has long strings
in it. It would be better to use std::string for name and surname.

append(datas);

Here you make a new node in the list that points to the "data"
struct you allocated above.
You do the same thing every loop, so all nodes will point to the
same one struct.

Probably you want to make a new "data" struct for each node, ie.
move the "datas = new data" line to be inside the loop.

}
file.close();
}

void main()

That is an error which your compiler should have diagnosed.
It should be:

int main()

 

[Vimal Aravindashan]

>
>
> drop this line, in next loop your py->next would be NULL
> and then you dererence NULL
>

i know i dont need to use link list but am a student and trying to
learn.
while(py!=NULL){//while py points sth reasonable

py->display();

py=py->next;
py->next=NULL;
}
yes here i print py first.it was first equal to head and by using next
pointer i am trying to print the items in the list till the tail so i
think it must bu true.

berkay, please quote the message you are replying to.

What Branimir Maksimovic was trying to tell you was what happens when
during the third iteration of the loop. First, py is equal to head.
After you display 'py', you set py to py->next, right? Let's call this
element(py->next) 'second'. second->next is currently 'third'. But when
you do py->next = NULL, you are setting second->next to NULL. Therefore,
after displaying 'second', when you do a py=py->next, you are setting py
to NULL. In the next statement, when you do py->next=NULL, you'll be
doing NULL->next, which is an error. But, your problems dont just stop
there. By setting py->next to NULL, you have lost all items starting
from the third one!

Cheers,
Vimal.

 

[berkay]

Thanks all for your answers i saw the mistakes i had done,i didnt
create an object for each node so it always used one object and i also
saw the mistake i had done in the display function , i ll of course use
a destructor to free the memory thaks all again...