This is so frustrating... Why does the 'head' change?

[Harry Potter]

This is a code for singly linked list, I wanted the value of 'head' to point to the head of the linked list. And it seems that i have coded in that fashion only, but the 'head' changes and goes to the tail, i dont know why. I even tried deleting all the 'tail' terms, but its the same result.

Please help!! some really small bug is there...

#include <iostream>
#include <vector>
#define Null 00


using namespace std;

class node{
public:
	int data;
	node *next;

	node(int n){
		data = n;
		next = Null;
	}
	node(){
		data = 9;
		next = Null;
	}
};

class SLL{
public:
	node *head, *tail;
	int count;

	SLL(){
		head = Null;
		tail = Null;
		count = 0;
	}

	void addNode(node node1){
		if(count == 0){
			head = &node1;
			tail = &node1;
			count++;
			cout << head->data << endl;
		}

		else{
			cout << head->data << endl;
			tail->next = &node1;
			tail = &node1;
			count++;
		}
	}
};


int main(){

	node myVector[4];
	for(int i = 0; i<4; i++){
		myVector[i].data = i+1;
	}
	SLL mySLL;

	mySLL.addNode(myVector[0]);
	mySLL.addNode(myVector[1]);
	mySLL.addNode(myVector[2]);
	//mySLL.addNode(myVector[3]);


	cout << mySLL.head->data << endl;

	return 0;
}

output:

1
2
3
3

[hanst99]

Why do you supply a default constructor for node when there is no sane default value for a node?

Anyways:
Why do you keep a pointer to tail? A tail is just what comes after head.

void addNode(node node1){
		if(count == 0){
			head = &node1;
			tail = &node1;

You're causing undefined behavior here, because you assign head and tail the memory address of something that won't exist anymore once the function returns.

Anyways: I suggest you to delete all of what you've written so far and start over.

[Harry Potter]

oh got it,...

i needed to accept the parameter by reference.

void addNode(node &node1)

You were right, thank you!

[hanst99]

No, that won't work either. That way, you have to keep the list in the same scope in which the nodes are created, which isn't very likely to be what you want (well I guess this is a homework so you probably don't care, but this would mean that the list is completely unusable).

[Harry Potter]

I din't understand what you are saying here, could you please elaborate?

As per the code, yes, i will have to create the node, and add it in the list in the same scope in which i declare the nodes.
But,
Then i can go ahead and send that SLL to other functions and use it. Because you said, i made a test function where the scope of the nodes is not there, but still the code works fine.

Or, are you pointing out something else?

#include <iostream>
#include <vector>
#define Null 00


using namespace std;

class node{
public:
	int data;
	node *next;

	node(){
		next = Null;
	}
};

class SLL{
public:
	node *head, *tail;
	int count;

	SLL(){
		head = Null;
		tail = Null;
		count = 0;
	}

	void addNode(node &newNode){

		if(count == 0){
			head = &newNode;
			tail = &newNode;
		}

		else{
			tail->next = &newNode;
			tail = &newNode;
		}

		count++;
	}

};

void testFunct(SLL &mySLL){
	cout << mySLL.head->next->data << endl;
}

int main(){

	vector <node> myVector;
	SLL mySLL;

	for(int i = 0; i<20; i++){
		node n1;
		n1.data = i*3;
		myVector.push_back(n1);
		mySLL.addNode(myVector[i]);
	}

	testFunct(mySLL);

	return 0;
}

[naraku9333]

What if another function needs to add a node to the list? Since you are allocating on the stack and just copying the address you will run into problems. I usually keep the node class internal to the list and just add the data. But you're method could work if you create a node and just copy the data (in addnode()).

[clanmjc]

This is what hanst99 was saying...

int main()
{
     //Moved to outer scope..
     SLL mySLL;
    {
         node myVector[4];
         for(int i = 0; i<4; i++)
         {
               myVector[i].data = i+1;
         }
    mySLL.addNode(myVector[0]);
    mySLL.addNode(myVector[1]);
    mySLL.addNode(myVector[2]);
    }

    //BOOOM, what does ->data reference?  myVector has been deconstructed when it 
    //left scope...
    cout << mySLL.head->data << endl;

    return 0;

}

It doesn't matter if you pass by reference or by value the way that addnode is implemented it does not own the memory it references. To properly do this, you allocate memory for a node and add the newly allocated memory, then your node pointers will own the memory. When you are done, you have to free that memory.

[clanmjc]

I think what you want is something like this.

void addNode(const node &newNode)
{
   //something like this...
   if(count == 0){
       head = new node(newNode);
       tail = head;
   }
   else{
       tail->next = new node(newNode);
       tail = tail->next;
   }
   
}

You will need to implement the copy constructor...

node& node(const node& rhs) : date(rhs.data), next(rhs.next){
    //Something like this...
}