-1

I am a beginner in programming. I am writing a sorted linked list in C++, I added three new functions, removeMiddleNode(), findWordStartWith(), and displayBackwards(). I built the solution and it says successful, however when I run the program everything also runs perfectly fine but it takes me to another file name xstring.cpp and reads "Exception thrown: read access violation. this was nullptr.". I do not understand what is wrong with my code, I would be grateful if someone could help/inform me on what this meant. It is my first-time learning C++ and writing on Visual Studio. Thank you. Below I have included my .h and .cpp file as well as my main.

HEADER FILE

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


//EXCULSIVE TO DATATYPE STRING
class node
{
public:
    string data;
    node* next;
};


class sortedRevLLStr
{
private:
    node* listData;
    int length;
    node* currentPos;

public:
    sortedRevLLStr(); //No arg constructor

    void makeEmpty(); 
    bool isFull();
    int getLength();
    bool findItem(string item);
    void putItem(string item);
    void deleteItem(string item);
    void resetList();
    string getNextItem();
    string findWordStartWith(char startAlphabet);
    bool removeMiddleNode();
    void displayListBackwards();
    void printList();
};

Here's the .cpp file.

#include "sortedRevLLStr.h"
#include <iostream>
#include <string>
using namespace std;


sortedRevLLStr::sortedRevLLStr()
{
    length = 0;
    listData = NULL;
    currentPos = NULL;
}

void sortedRevLLStr::makeEmpty()
{
    node* tempPtr;
    while (listData != NULL)
    {
        tempPtr = listData;
        listData = listData->next;
        delete tempPtr;
    }
    length = 0;
}

bool sortedRevLLStr::isFull()
{
    node* location;
    try
    {
        location = new node;
        delete location;
        return false;
    }
    catch (std::bad_alloc exception)
    {
        return true;
    }
}

int sortedRevLLStr::getLength()
{
    return length;
}

bool sortedRevLLStr::findItem(string item)
{
    bool moreToSearch;
    node* location;
    location = listData;
    bool found = false;
    moreToSearch = (location != NULL);
    while (moreToSearch && !found)
        if (item == location->data)
        {
            found = true;
            break;
        }
        else if (item > location->data)
        {
            location = location->next;
            moreToSearch = (location != NULL);
        }
        else
        {
            moreToSearch = false;
            break;
        }
    return found;
}

void sortedRevLLStr::putItem(string item)
{
    if (isFull())
        return;

    node* location;
    node* prevLoc;
    node* newNode;
    bool moreToSearch;
    location = listData;
    prevLoc = NULL;
    moreToSearch = (location != NULL);

    while (moreToSearch)
    {
        if (item < location->data) // change to descending order
        {
            prevLoc = location;
            location = location->next;
            moreToSearch = (location != NULL);
        }
        else if (item >= location->data) // change to descending order
        {
            moreToSearch = false;
            break;
        }
    }
    newNode = new node;
    newNode->data = item;
    if (prevLoc == NULL)
    {
        newNode->next = listData;
        listData = newNode;
    }
    else
    {
        newNode->next = location;
        prevLoc->next = newNode;
    }
    length++;
}

void sortedRevLLStr::deleteItem(string item)
 {
    //resetList();
    node* location;
    node* prevLoc;
    node* tempLocation = NULL;
    location = listData;
    prevLoc = NULL;
    bool moreToSearch = (location != NULL);
    bool found = false;

    if (item == location->data)
    {
        found = true;
        tempLocation = location;
        listData = listData->next;
    }
    else
    {
        while (moreToSearch)
        {
            if (item == location->data)
            {
                found = true;
                moreToSearch = false;
                tempLocation = location;
                prevLoc->next = location->next;
            }
            else if (item > location->data)
            {
                prevLoc = location;
                location = location->next;
                moreToSearch = (location != NULL);
            }
            else
            {
                moreToSearch = false;
                break;
            }
        }
    }
    if (found)
    {
        delete tempLocation;
        length--;
    }
}

void sortedRevLLStr::resetList()
{
    currentPos = NULL;
}

string sortedRevLLStr::getNextItem()
{
    if (currentPos == NULL)
        currentPos = listData;
    else
        currentPos = currentPos->next;
    return currentPos->data;
}

string sortedRevLLStr::findWordStartWith(char a)
{

    node* temp = listData; 
    string str; 
    resetList();

    while(temp != NULL) 
    {
        for (int i = 0; i < length; i++) 
        {
            str = getNextItem(); 
            char startWith = str[0]; 

            if (startWith == a)
                return str;
        }
        return "Not Found";  
        cout << endl;
    }
    return "";
}

bool sortedRevLLStr::removeMiddleNode()
{
    int length = getLength();
    int getmiddleNode = length / 2;
    int deleteMidNode;

    if (listData == NULL) 
    {
        return false;
    }
    if (length < 2)  
    {
        return false;
    }
    else 
    {

        if (length % 2 == 0) 
            deleteMidNode = getmiddleNode - 1;
        else
            deleteMidNode = getmiddleNode; 
        node* prevNode = NULL;   // set to NULL
        node* midNode = listData;


        for (int i = 0; i < deleteMidNode; i++)
        {
            prevNode = midNode;
            midNode = midNode->next;
        }
        prevNode->next = midNode->next; 
        delete midNode; 
    }
    length--;
    return true; 
}

void sortedRevLLStr::displayListBackwards()
{
    resetList();
    node* currentPos = listData; 
    node* prevPtr = NULL;
    node* temp = NULL;

    while (currentPos != NULL) 
    {   
        temp = currentPos->next;    
        currentPos->next = prevPtr;
            
        prevPtr = currentPos;
        currentPos = temp;
    }
    listData = prevPtr;
}

void sortedRevLLStr::printList()
{
    string item;
    resetList();
    for (int i = 0; i < length; i++)
    {
        item = getNextItem();
        cout << item << " ";
    }
    cout << endl;
}

MAIN.CPP

#include "sortedRevLLStr.h"
#include <iostream>
#include <string>
using namespace std;

int main()
{
    sortedRevLLStr list = sortedRevLLStr();
    string inp1, inp2, inp3, inp4, inp5;
    cout << "Please enter 5 strings to enter in the list: ";
    cin >> inp1;
    cin >> inp2;
    cin >> inp3;
    cin >> inp4;
    cin >> inp5;
    list.putItem(inp1);
    list.putItem(inp2);
    list.putItem(inp3);
    list.putItem(inp4);
    list.putItem(inp5);
    list.printList();

    list.displayListBackwards();
    list.printList();

    string inp6;
    cout << "Pick an item you would like to delete from the list: ";
    cin >> inp6;
    list.deleteItem(inp6);
    list.printList();

    cout << "Enter a char to find the word in the list" << endl;
    char sChar;
    cin >> sChar;
    cout << list.findWordStartWith(sChar) << endl;
    list.printList();

    list.removeMiddleNode();
    list.printList();
    return 0;
}
  • 1
    Learning to use your debugger can help you quickly hone in on where the issue is. I imagine you could fix this completely yourself with a stack trace to show you how the code arrived at the error. – sweenish Oct 07 '22 at 23:08
  • 1
    I refuse to believe this is a minimal reproducible example. – Taekahn Oct 07 '22 at 23:10
  • You'll be glad to hear you don't need anyone's help to figure this out, just a tool you already have: your debugger! This is exactly what a debugger is for. It [runs your program, one line at a time, and shows you what's happening](https://stackoverflow.com/questions/25385173/), this is something that's every C++ developer must know how to do. With your debugger's help you'll able to quickly find all problems in this and all future programs you write, without having to ask anyone for help. Have you tried using your debugger, already? If not, why not? What did your debugger show you? – Sam Varshavchik Oct 07 '22 at 23:11
  • At a quick glance, I see that nodes, an implementation detail of the list, is globally exposed. Constructors don't use initialization sections. You aren't using default member initialization, `currentPos` seems utterly pointless. Your linked list should be, but is not adhering to the Rule of 5. `isFull()` is unnecessary. `getLength()` should be const. Your find function is needlessly complicated. `removeMIddleNode()` is pointless; make a remove function that can remove any node. You re-write your find logic over and over instead of using the find function. – sweenish Oct 07 '22 at 23:13
  • Printing a singly linked list backwards is also pointless. There could be some merit in reversing the list in place, but just printing it backwards? Again, though, this is a perfect time to learn to use your debugger. – sweenish Oct 07 '22 at 23:21
  • Oh, and `using namespace std;` is generally a bad idea, but it's a horrific idea in a header. – sweenish Oct 07 '22 at 23:27
  • My final comment will be that a sorted linked list would actually be a ton easier as a doubly linked list. – sweenish Oct 07 '22 at 23:29
  • I lied about the last comment being my final one. Instead of saying "I run it and it crashes," which does nothing to narrow down the scope of the issue, be specific. After deleting an item from the list and printing it is when it crashes. Even without a debugger, you've now narrowed down the issue (likely) to your deletion code. And instead of forcing anyone to type stuff out every time the program is run, just hardcode some stuff while you're testing. – sweenish Oct 07 '22 at 23:37

1 Answers1

0

Because I am currently stuck in bed, I have the time to do the digging you didn't do. You simply said the program crashes.

Specifically, it crashes after deleting an item from the list and attempting a print. Deletion is a pretty common spot for issues when people write linked lists.

Here's your function:

void sortedRevLLStr::deleteItem(string item)
 {
    //resetList();
    node* location;
    node* prevLoc;
    node* tempLocation = NULL;
    location = listData;
    prevLoc = NULL;
    bool moreToSearch = (location != NULL);  // Completely unnecessary
    bool found = false;

    // No empty check, so this can attempt to dereference a null pointer
    if (item == location->data)
    {
        found = true;
        tempLocation = location;
        listData = listData->next;
    }
    else
    {
        while (moreToSearch)
        {
            // Checks the same node twice at start
            if (item == location->data)
            {
                found = true;
                moreToSearch = false;
                tempLocation = location;
                prevLoc->next = location->next;
            }
            else if (item > location->data)
            {
                prevLoc = location;
                location = location->next;
                moreToSearch = (location != NULL);
            }
            else
            {
                moreToSearch = false;
                break;
            }
        }
    }
    // This is your problem right here
    if (found)
    {
        delete tempLocation;
        length--;
    }
}

The issue that you delete a node, but do nothing to ensure that your list remains intact. So, when you go to print your list, the node previous to what was deleted has a dangling pointer, and you get your crash. You need to ensure that your linked list stays linked.

Before you delete your node, ensure the list is intact by reassigning pointers.

prev->next = location->next;`

Here's a reworked delete function.

void sortedRevLLStr::deleteItem(string item)
{
  node* walker = listData->next;
  node* prev = listData;

  // Account for the head of the list
  if (prev->data == item)
  {
    node* tmp = listData;
    listData = listData->next;
    delete tmp;

    return;
  }

  // It would be better to use your find() function here instead of re-writing
  // your logic, but you'd need a private find that returns a node*, or
  // a std::pair<node*, node*> for the previous and actual nodes, and I don't
  // want to be bothered.
  while (walker) {
    if (walker->data == item) {
      prev->next = walker->next;
      delete walker;
      --length;

      return;
    } else if (item > walker->data) {
      prev = walker;
      walker = walker->next;
    } else {
      return;
    }
  }
}

In the future, I would advise you to not waste people's time by not providing pertinent information. I happened to have time to spare, but this behavior will not fly in the workplace.

http://www.gregcons.com/KateBlog/HowToAskForCCodingHelp.aspx

sweenish
  • 4,793
  • 3
  • 12
  • 23
  • Thank you for helping me out. I'm sorry that my lack of knowledge was an inconvenience, I'm learning as I go and want to understand my problem from an experienced perspective. I am grateful that you've taken time out of your day to help me and once I post another question, I will be clearer. On the contrary since this is only my 3rd week of learning C++ and tackling pointers, I've spent a week on this and at the end I gave up and posted my question on here. Again thank you for your help. – Alexa Demao Oct 08 '22 at 00:14
  • It wasn't lack of knowledge. It was lack of information in the question that you had. You knew *when* your program crashed and withheld the information. I'll also say that if you're really that early in your programming, that a linked list is a terrible idea. Linked lists require the application of *many* principles, and are a project whose completion I would mark as graduating out of being a beginner. Biting off more than you can chew leads to a poor learning experience. – sweenish Oct 08 '22 at 00:17
  • I will let my school's computer science department know. I'm just as pissed as you are. Thank you again! – Alexa Demao Oct 08 '22 at 01:22