constructor for linked list classes?

BossArky

Hello, this program compiles ok...its supposed to create a linked list of numbers input from the user...but when it gets the first number I get a popup error message saying that there is an

"unhandled exception in the .exe 0xC0000005: Access Violation"

I think it is because I have no proper constructers...I'm not too sure about the whole constructor concept...do I need two of them here for my code?..what should they be? I have commented out two that I had created...didn't think they were correct. Thanks.


#include<iostream.h>

class node
{
private:
friend class llist; //llist class to manipulate nodes, and to access node.
int information;
node * previous;
node * next;
};

class llist
{
private:
node base; //a base node, from which other nodes will spring.
node * current; //pointer to the current node.

public:
void push(int item);
int pop();
};

//CONSTRUCTORS?...I think these are wrong..compiles ok with them...but still get access violation
//node::node();
//llist::llist();


int main()
{


llist linked_list;

cout<<"Hello!"<<endl;
cout<<"Enter data: "<<endl;
int data;
while(data != EOF)
{
cin>>data;
linked_list.push(data);
}

return 0;
}


void llist::push(int item) //boards.ie making fun of push function
{
node * temp = current;
current->next = new node;
current = current->next;
current->information = item;
current->previous = temp;
}

int llist::pop() //boards.ie making fun of pop function
{
int temp;
temp = current->information;
if(current->previous = NULL)
{
return temp;
}
current->previous = current;
delete current->next;
return temp;

}

hussey

http://www.geocities.com/sketerpot/lltut.html

if your going to yoink code somewhere it would be better to yoink full code no?

have a look at this .. it is a linked list of boxes
http://www.cs.uregina.ca/dept/manuals/C++/objlink.cpp

DeadBankClerk

[php]void llist::push(int item) //boards.ie making fun of push function
{
node * temp = current;
current->next = new node; /* The Error is here */
current = current->next;
current->information = item;
current->previous = temp;
}[/php]

You are never assigning the pointer 'current' to any value before you dereference it. It is pointing into a random location in memory.

Silent Bob

You should use ctors for stuff like this, it's good practice to NULLify pointers when you instantiate classes.

BossArky

ompiling...
practice_classes.cpp
C:\Program Files\Microsoft Visual Studio\MyProjects\practice_classes\practice_classes.cpp(56) : error C2664: 'push' : cannot convert parameter 1 from 'char' to 'char []'
Conversion from integral type to pointer type requires reinterpret_cast, C-style cast or function-style cast
C:\Program Files\Microsoft Visual Studio\MyProjects\practice_classes\practice_classes.cpp(79) : error C2440: '=' : cannot convert from 'char []' to 'char [20]'
There are no conversions to array types, although there are conversions to references or pointers to arrays
C:\Program Files\Microsoft Visual Studio\MyProjects\practice_classes\practice_classes.cpp(80) : error C2440: '=' : cannot convert from 'char []' to 'char [20]'
There are no conversions to array types, although there are conversions to references or pointers to arrays
Error executing cl.exe.

---

#include<iostream.h> 

class node 
{ 
private: 
    friend class llist;      //llist class to manipulate nodes, and we want it to access node. 
    //int information; 
    
	char name[20];
	char number[20];
	
	node * previous; 
    node * next; 
public:
	node();
}; 

class llist 
{ 
private: 
	node * base;            //a base node, from which other nodes will spring...node base
    node * current;      //pointer to the current node. 
	  

public: 
    void push(char name[], char number[]); 
    int pop(); 
	llist();
}; 

node::node()				//construct of node
{ 
	previous = NULL; 
	next = NULL; 
	//information = 0; 
	//name = NULL;
	//number = NULL;
} 

llist::llist()				//constructor for llist
{ 
	current = new node(); 
} 

int main() 
{ 
    llist chain;            //chain is what I'm declaring llist as. 
		
    cout<<"Hello!"<<endl; 
    cout<<"Enter data: "<<endl; 
    char name, number; 
    while((name != EOF) || (number != EOF)) 
    { 
    cin>>name;
	cin>>number;
    chain.push(name, number);
	//cout<<chain.pop()<<endl;
    }

Dizz

Tut tut - your mixing your types there... tryin to push a char into an array of chars. Your code expects an array so give it an array not a char! Make a templated linked list (<nasty hack>hee hee of void* </nasty hack>) and you'll avoid this problem (so long as you don't forget what type your linked list is of!)

Dizz

BossArky

Ok thanks...I was practising with that trying to understand what is going on.

This next program is what I'm really trying to get working. What is wrong with the pop function?

All I want to do is take in a few numbers..store them in a linked list of classes..then pop them out after the user signifes through a -1 that they are finished. If you could tell me exactly where in the code to fix it, that would be handy.

d

#include<iostream.h> 
#include<conio.h>

class node 
{ 
private: 
    friend class llist;      //llist class to manipulate nodes, and we want it to access node. 
    
	char information; 
    
	node * previous; 
    node * next; 
public:
	node();
}; 

class llist 
{ 
private: 
	node * base;            //a base node, from which other nodes will spring...node base
    node * current;      //pointer to the current node. 
	  

public: 
    void push(int item); 
    int pop(void); 
	llist();
}; 

node::node()				//construct of node
{ 
	previous = NULL; 
	next = NULL; 
	information = 0;
} 

llist::llist()				//constructor for llist
{ 
	current = new node(); 
} 

int main() 
{ 
    llist chain;            //chain is what I'm declaring llist as. 
		
    cout<<"Hello!"<<endl; 
    cout<<"Enter data: "<<endl; 
	cout<<EOF<<endl;
    int data;
    while(data != EOF) 
    { 
	cin>>data;
	chain.push(data);
	} 

	cout<<chain.pop()<<endl;
	
getch();

return 0; 
} 


void llist::push(int item) 
{ 
    node * temp = current; 
	current->next = new node; 
    current = current->next; 
    current->information = item;
    current->previous = temp; 
	
} 

int llist::pop() 
{ 

	
	while(current->information != NULL)
	{
	current = current->previous;
	}

	while(current->information != NULL)
	{
	cout<<current->information<<endl;
	current = current->next;

	}
	
/*
    int temp; 
    temp = current->information; 
		
		if(current->previous = NULL) 
        { 
        return temp; 
        } 
	
	current->previous = current; 
    delete current->next; 
    return temp; 
*/
	return 1;
} 

Silent Bob

For a start the semantics of your pop function are all wrong.

What you have:

while(current->information != NULL)
{
	current = current->previous;
}

while(current->information != NULL)
{
	cout<<current->information<<endl;
	current = current->next;
}

The first loop will continue to iterate until it finds a node with 0 as it's information. After the first loop current is set to a node with 0 as it's information and so the second loop will never be entered.

That's not the point though. A pop from a linked list should only remove ONE node and it should return the value stored at that node.
The only case you have to worry about is when the linked list is empty, otherwise you just remove a node and return its information.

You should have code in your main function along the lines of

while(linked-list not empty)
{
    cout << link-list.pop() << endl;
}

if you want to pop off every node.

Talliesin

Originally posted by Silent Bob
A pop from a linked list should only remove ONE node and it should return the value stored at that node.

A minor point perhaps, but pop's don't always return a value.

This would be a very minor point, but for the fact that the STL containers (the std::list, std::vector and std::stack templates etc.) are examples of such containers, and since that's what you would use for most real code that needed lists it's important not to have the idea that pops always return a value too deeply ingrained into your thinking.

The reason it doesn't return a value is that if it was used with a type with complicated and expensive copy semantics it might not be possible to optimise away the overhead incurred in a blind pop (that is a pop where you don't care what the value is), whereas a "normal" pop can always be created efficiently from a peek followed by a pop that doesn't return the value.

Silent Bob

Originally posted by Talliesin
A minor point perhaps, but pop's don't always return a value.

This would be a very minor point, but for the fact that the STL containers (the std::list, std::vector and std::stack templates etc.) are examples of such containers, and since that's what you would use for most real code that needed lists it's important not to have the idea that pops always return a value too deeply ingrained into your thinking.

Good point