Appending Node to Doubly Linked List

[Speakmore]

Hey there, can someone please help me with this? I'm trying to append a node to a doubly linked list. It seems to crash inside the while loop after doing temp=temp->next
Thanks a bunch!

void DLL::append(string ss, string name, int & count){
	
	Node *temp;
	Node *newNode = new Node();
	newNode->ssn = ss;
	newNode->name = name;
	newNode->next= NULL;
	newNode->prev = NULL;
	temp=headPtr;
	
	if(headPtr == NULL){
				
	  headPtr = newNode;	
	  count++;	
	  return;
	
	}else{
					
	  while(temp->next != NULL){			
	    temp = temp->next;						
	  }
			
	  newNode->prev = temp;
	  newNode->next = NULL;
	  temp->next = newNode;
	  count++;	
	}
}

It gathers the following:

20 John
30 Mike
40 Kate

[2kaud]

How does 'it seem to crash' Have you traced through execution using the debugger? Where is headPtr set to NULL?

Note that pointers should now be set to nullptr rather than NULL.

Consider

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

struct Node {
	string ssn;
	string name;
	Node* next;
	Node* prev;
};

Node* headPtr = nullptr;

void Append(string ss, string name, int& count)
{
	Node *temp = headPtr;
	Node *newNode = new Node();
	newNode->ssn = ss;
	newNode->name = name;
	newNode->next = nullptr;
	newNode->prev = nullptr;

	if (headPtr == nullptr) {
		headPtr = newNode;
		count++;
		return;
	}

	while (temp->next != nullptr)
		temp = temp->next;

	newNode->prev = temp;
	temp->next = newNode;
	count++;
}

int main()
{
	int count = 0;
	Append("20", "John", count);
	Append("30", "Mike", count);
	Append("40", "Kate", count);

	for (Node* np = headPtr; np; np = np->next)
		cout << np->ssn << " " << np->name << endl;
}

Note this example doesn't free allocated memory.

[monarch_dodra]

I'd suggest you start by having a Node constructor that at the very least initializes your fields for you (especially the pointers). Better yet, use C++11's member initialiser.

Anyways, a "pro tip" for doubly linked list is to make them circular, and have a "center sentinel node". This avoid having to handle null pointers at all, and eliminates most needs for any conditional testing too.

This sample code does it. It's provided with a visitor, as it is simpler to implement than an iterator, but ideally, you'd want to provide iterators.

#include <iostream>
#include <functional>
#include <string>
using namespace std;

struct DLL {
  private:
    struct Node {
    	string ssn;
    	string name;
    	Node* next = nullptr;
    	Node* prev = nullptr;
    };
    Node sentinel;

  public:
    DLL() {
        sentinel.next = &sentinel;
        sentinel.prev = &sentinel;
    }

    void Append(string ssn, string name, int& count) {
        Node* new_guy = new Node();
        new_guy->ssn = ssn;
        new_guy->name = name;
        ConnectNode(sentinel.prev, new_guy);
        ConnectNode(new_guy, &sentinel);
        ++count;
    }

    void Visit(function<void(string, string)> visitor) {
        for (Node* p = sentinel.next; p != &sentinel; p = p->next) {
            visitor(p->ssn, p->name);
        }
    }

  private:
    void ConnectNode(Node* lhs, Node* rhs) {
        lhs->next = rhs;
        rhs->prev = lhs;
    }
};

int main()
{
    int count = 0;
    DLL dll;
	cout << "The list contains " << count << " entries:" << endl;
	dll.Visit([](string ssn, string name) {cout << ssn << " " << name << endl;});
	dll.Append("20", "John", count);
	dll.Append("30", "Mike", count);
	dll.Append("40", "Kate", count);
	cout << "The list contains " << count << " entries:" << endl;
	dll.Visit([](string ssn, string name) {cout << ssn << " " << name << endl;});
}

This code only contains append, but implementation for all other list primitives are just as trivial.

[monarch_dodra]

Also, just looked at your code again: One of the main points of a doubly linked list is backwards iteration and being able to access the last element easily. You should NOT be walking your list to find the last element. That is a major fail.