Reputation: 23

Single Linked List not working (C++)

The following code builds correctly but causes the program to crash when I run it. Can someone please tell me whats wrong with it. I suspect that there is something wrong with the DeleteNode function.

#include <iostream>
#include <cstdlib>

using namespace std;

class list {
private:
    typedef struct node {
        int data;
        node* next;
    }* nodePtr;  //this means that 'nodePtr' will mean a pointer to the struct node

nodePtr head;
nodePtr current;
nodePtr temp;

public:
list() {  //constuctor
    head = NULL;
    current = NULL;
    temp = NULL;
};

void AddNode(int addData)  //to add a particular data value
{
    nodePtr n= new node;
    n->next = NULL;
    n->data = addData;

    if (head != NULL) {  //if a list is already set up
        current = head;
        while (current->next != NULL) {  //to get to the last node in the list
            current = current->next;
        }
        current->next = n;
    }
    else {  // if list is not created
        head = n;  //new node is front of the list
    }
}

void DeleteNode(int delData)  //to delete a particular data value
{
    nodePtr delPtr = NULL;
    temp = head;
    current = head;

    while (current != NULL  && current->data!=delData) {  //pass through whole list && find value
        temp = current;
        current = current->next;
    }

    if (current = NULL) {  //data value not found in list
        cout << delData << " was not in the list." << endl;
        delete delPtr;  //to free up memory space
    }
    else {
        delPtr = current;
        current = current->next;
        temp->next = current;  //to reconnect list

        if (delPtr == head) {
            head = head->next;
            temp = head;
        }

        delete delPtr;
        cout << "The value " << delData << "was deleted." << endl;
    }
}

void PrintList()  //to print all the data values
{
    current = head;

    while (current != NULL) {  //to go through the data valued of the list
        cout << current->data << endl;
        current = current->next;
    }
}

};



int main()
{
    list Shahzad;

    Shahzad.AddNode(2);
    Shahzad.AddNode(78);
    Shahzad.AddNode(28);
    Shahzad.AddNode(2398);

    Shahzad.DeleteNode(78);
    Shahzad.PrintList();
    return 0;
}

Upvotes: 2

Answers (4)

Prashanth

Reputation: 3

Apart from all the suggestions here, you can use some safe programming practices to catch bugs early.

For ex: you wrote

if (current = NULL)

Instead, try writing the value being checked on the LHS and the variable on the RHS like this:

if ( NULL == current)

Here, if you mistyped

if (NULL = current)

the compiler will complain. You have a compile time bug now instead of a run-time one. This is far easier to find and debug.

Upvotes: 0

Ziezi

Reputation: 6467

Firstly, few code and file management remarks: consider separating your code into .h file where class members are declared and .cpp where class members are implemented, this will make your class easy to comprehend and possible errors will be easier to locate.

Secondly, a general advice when dealing with structures containing pointers is attention to proper resource management, i.e. pointer definitions, initialisations and deletions should be dealt with caution. If you are novice, consider the use of already provided smart pointer facilities like: std::unique_ptr which will "retain sole ownership of an object through a pointer and destroys that object when the unique_ptr goes out of scope"

Thirdly, use debugger to get rid of trivial errors like:

if (current = NULL)

which by the way contains additional inaccuracy expressed in the use of NULL instead of the pointer literal nullptr.

Lastly, check each of the member functions separately after you finish the initial implementation and only then proceed with further class expansion, otherwise you risk the accumulation of errors from multiple sources which will make your job very difficult

Upvotes: 1