Error while trying link previous node to the node after the node to be deleted

Question

For this assignment I'm supposed to create a linked list with a insert function, a remove function, and a reverse function. It uses an input file to get the values for the list and uses another input file for values to be removed. My insert and reverse functions work, but my remove function gives me an error in the else statement when I try to link the previous node to the node after nodePtr. It says potentially uninitialized local pointer variable 'previousNodePtr' used. I'm not sure what I'm doing wrong to relink my list so that I can remove the desired node. You can find my problem in the remove function about 90 lines down. Here's my code:

//use a linked list to maintain a sorted list of numbers in descending
//order
#include <iostream>
#include <fstream>
#include <string>
#include <cstdlib>
using namespace std;

const int ARRAY_SIZE = 100;

//input the data from a file to an array A, n is used to calculate the total number of
//data in the file
void input(int A[], int & n, const string & fileName) 
{
    ifstream inputFile;
    int value;

    inputFile.open(fileName.c_str());

    if (!inputFile)
    {
        cout << "Error opening the file " << fileName << endl;
        exit(0);
    }

    //read data until the end of the file and calculate n
    while(inputFile >> value)
    {
        A[n] = value;
        n++;
    }

    inputFile.close();
}


struct ListNode
{
    int value;
    ListNode *next;
    //struct constructor
    ListNode (int input_value, ListNode * input_next = NULL)
    {
        value = input_value;
        next = input_next;
    }
};

//define a class for the linked list
class LinkedList
{
    private:
        ListNode *head;// the head of the linked list

    public:
        LinkedList() {head = NULL;}
        ~LinkedList();
        void insert(int x);// add a number x the end of the list
        void remove(int x);//remove x from the list
        void reverse();//reverse the list
        void printListToFile(const string & fileName) const; // print the     list to a file
        void printList() const; // print the list to the console (computer's screen), this function may be helpful when you test your program
};

//insert
void LinkedList::insert(int x)
{   
    ListNode *nodePtr, *previousNodePtr;
    if (head == NULL || head->value <= x)
    {
        head = new ListNode(x, head);
    }
    else
    {
        previousNodePtr = head;
        nodePtr = head->next;

        while (nodePtr != NULL && nodePtr->value > x)
        {
            previousNodePtr = nodePtr;
            nodePtr = nodePtr->next;
        }

        previousNodePtr->next = new ListNode(x, nodePtr);
    }

}

//remove
void LinkedList::remove(int x)
{
    ListNode *nodePtr, *previousNodePtr;
    if (!head) return;
    if (head->value == x)
    {
        nodePtr = head;
        head = head->next;
        delete nodePtr;
    }
    else
    {
        nodePtr = head;
        while (nodePtr != NULL && nodePtr->value != x)
        {
            previousNodePtr = nodePtr;
            nodePtr = nodePtr->next;
        }
        if (nodePtr)
        {
            previousNodePtr->next = nodePtr->next;
            delete nodePtr;
        }

    }
}

//reverse
void LinkedList::reverse()
{
    ListNode *nodePtr, *previousNodePtr, *nodeNext;

     nodePtr = head;
     nodeNext = NULL;
     previousNodePtr = NULL;

    while (nodePtr != NULL)
    {
        nodeNext = nodePtr->next;
        nodePtr->next = previousNodePtr;
        previousNodePtr = nodePtr;
        nodePtr = nodeNext;
    }
    head = previousNodePtr;
}

void LinkedList::printList() const
{
    ListNode *p = head;
    while (p != NULL)
    {
        if (p->next == NULL)
            cout << p->value << endl; 
        else
            cout << p->value << " -> ";
        p = p->next;
    }
}

void LinkedList::printListToFile(const string & fileName) const
{
    ofstream outputFile;
    outputFile.open(fileName.c_str());
    if (!outputFile)
    {
        cout << "Error opening the file " << fileName << endl;
        exit(0);
    }

    ListNode *p = head;
    while (p != NULL)
    {
        if (p->next == NULL)
            outputFile << p->value << endl; 
        else
            outputFile << p->value << " -> ";
        p = p->next;
    }

    outputFile.close();
}

//destructor, delete all nodes
LinkedList::~LinkedList()
{
    ListNode *p = head;
    while (p != NULL)
    {
        ListNode * garbageNode = p;
        p = p->next;
        delete garbageNode;
    }
}

int main()
{
    int A[ARRAY_SIZE];// store numbers for insert
    int B[ARRAY_SIZE]; // store numbers for remove
    int n = 0;// the number of elements that will be stored in A
    int m = 0;// the number of elements that will be stored in B
    string inputFile1 = "hw2_Q1_insertData.txt"; // file with data for     insert 
    string inputFile2 = "hw2_Q1_removeData.txt"; // file with data for     remove 

    //input data from the two files
    input(A, n, inputFile1); 
    input(B, m, inputFile2); 

    LinkedList list;

    //do insert operations
    for (int i = 0; i < n; i++)
        list.insert(A[i]);

    //do remove operations
    for (int i = 0; i < m; i++)
        list.remove(B[i]);

    //list.printList();

    //reverse the list
    list.reverse();

    list.printList();

    //print the list to the output file
    string outputFile = "hw2_Q1_output.txt"; // the output file 
    list.printListToFile(outputFile);

    return 0;
}

Answer 1

In this case, the compiler is being overly careful - it is warning you that there might be a path through your program where previousNodePtr is not set to a value.

As you are checking for head matching the value and returning earlier, you will always go through the loop at least once, so previousNodePtr will always be set.

One way to fix this would be to set previousNodePtr to NULL when you declare it - this will get rid of the warning.

A better way would be to realise you don't need to test against the head node again in your loop. You can then do something like this:

...
else
{
    ListNode *previousNodePtr = head;
    nodePtr = head->next;
    while (nodePtr != NULL && nodePtr->value != x)
    {
        previousNodePtr = nodePtr;
        nodePtr = nodePtr->next;
    }
    if (nodePtr)
    {
        previousNodePtr->next = nodePtr->next;
        delete nodePtr;
    }

Also note that I moved the declaration of previousNodePtr closer to where it was used.