0

Node.h

#pragma once
#include <iostream>
#include<memory>


class Node
{
public:
    Node();
    Node(int k, int d);
    int key;
    int data;

    
    std::shared_ptr<Node> next;
    std::shared_ptr<Node> previous;
    //Node* next;
    //Node* previous;
};

doubleLinkedList.h

#pragma once

/*! \class Double Linked List
    \brief A double linked list data structure
*/

#include <iostream>
#include "../Node.h"
#include <string>
#include<memory>



class DoubleLinkedList : public Node
{
private :
    std::shared_ptr<Node> head;             //Node* head; 
    std::shared_ptr<Node> temp;           //Node* temp;
    std::shared_ptr<Node> mypointer;            //Node* ptr;
    std::shared_ptr<Node> nextNode;
    std::shared_ptr<Node> prevNode;

public :
    DoubleLinkedList();
    DoubleLinkedList(std::shared_ptr<Node> n);
    std::shared_ptr<Node> checkNodeExsits(int k); //Node*
    void addNodeToFront(std::shared_ptr<Node> n); //Node*
    void addNodeToEnd(std::shared_ptr<Node>  n);   //Node*
    void addNodeAfter(int k, std::shared_ptr<Node> n); //Node*
    void UpdateNode(int k , int d);
    void deleteNode(int k);
    void printList();
    void printInfo(std::string Info);



};

Node.cpp

#include "Node.h"
#include <iostream>



Node::Node() 
{
    key = 0;
    data = 0;
    next = nullptr;
    previous = nullptr;
}


Node::Node(int k, int d) 
{
    key = k;
    data = d;
}

doubleLinkedList.cpp

#include <iostream>
#include "include\doubleLinkedList.h"



DoubleLinkedList::DoubleLinkedList()
{
    head = nullptr;
}


DoubleLinkedList::DoubleLinkedList(std::shared_ptr<Node> n)
{
  
     head = n;
}

std::shared_ptr<Node> DoubleLinkedList::checkNodeExsits(int k)
{
     temp = nullptr;
     mypointer = head;

    while (mypointer != nullptr) {
        if (mypointer -> key == k) {
            temp = mypointer;
        }
        
        mypointer = mypointer-> next;

    }

    return temp;
}


void DoubleLinkedList::addNodeToFront(std::shared_ptr<Node> n)
{
    if (checkNodeExsits(n->key) != nullptr) 
    {
        
        printInfo("Node Already exist with key Number ");
    }
    else {
        if (head == nullptr) {
            head = n;
            printInfo("Node Added as Head Node");
        }
        else {
            head->previous = n;
            n->next = head;
            head = n;
            printInfo("Node Added To The Begining");
        }

    }
}


void DoubleLinkedList::addNodeToEnd(std::shared_ptr<Node> n)
{

    if (checkNodeExsits(n->key) != nullptr) 
    {
        printInfo("Node Already exist with key Number");
    }
    else {
        if (head == nullptr) 
        {
            head = n;  // if there isnt any node in the list.
            printInfo("Node Has Been Added As Head Node");
        }
        else {
            mypointer = head;
            while (mypointer ->next != nullptr)
            {
                mypointer = mypointer->next;
            }
            mypointer->next = n;
            n->previous = mypointer;
            printInfo("Node Has Been Added To The End");
        }
    }

}


void DoubleLinkedList::addNodeAfter(int k, std::shared_ptr<Node> n)
{
    mypointer = checkNodeExsits(k);
    if (mypointer == nullptr) {
        printInfo("No Node Exists With The Key Value");
    }
    else {
        if (checkNodeExsits(n -> key) != nullptr) {
            printInfo("Node Already exist with key Number");
        }
        else {
               nextNode = mypointer-> next;
            // inserting at the end
            if (nextNode == nullptr) {
                mypointer-> next = n;
                n -> previous = mypointer;
                printInfo("Node Inserted at the END");
            }

            //inserting in between
            else {
                n -> next = nextNode;
                nextNode -> previous = n;
                n -> previous = mypointer;
                mypointer-> next = n;
                printInfo("Node Inserted in Between");

            }

        }
    }
}


void DoubleLinkedList::UpdateNode(int k, int d)
{
    mypointer = checkNodeExsits(k);
    if (mypointer != nullptr) {
        mypointer-> data = d;
        std::cout << "Node Data Updated Successfully" << std::endl;
    }
    else {
        std::cout << "Node Doesn't exist with key value : " << k << std::endl;
    }
}


void DoubleLinkedList::deleteNode(int k)
{
    mypointer = checkNodeExsits(k);
    if (mypointer == nullptr) {
        std::cout << "No node exists with key value: " << k << std::endl;
    }
    else {

        if (head -> key == k) {
            head = head -> next;
            std::cout << "Node UNLINKED with keys value : " << k << std::endl;
        }
        else {
             nextNode = mypointer-> next;
             prevNode = mypointer-> previous;
            // deleting at the end
            if (nextNode == nullptr) {

                prevNode -> next = nullptr;
                std::cout << "Node Deleted at the END" << std::endl;
            }

            //deleting in between
            else {
                prevNode -> next = nextNode;
                nextNode -> previous = prevNode;
                std::cout << "Node Deleted in Between" << std::endl;

            }
        }
    }
}


void DoubleLinkedList::printList()
{
    if (head == nullptr) {
        std::cout << "No Nodes in Doubly Linked List";
    }
    else {
        std::cout << std::endl << "Doubly Linked List Values : ";
         temp = head;

        while (temp != nullptr) {
            std::cout << "[Key: " << temp->key << ", Data: " << temp->data << "] <___> " << std::endl;
            temp = temp -> next;
        }
    }
}


void DoubleLinkedList::printInfo(std::string Info)
{
    std::cout << Info << std::endl;
}

main.cpp

#include <iostream>
#include "../include/doubleLinkedList.h"
#include"../Node.h"

void Print(std::string info)
{
    std::cout << info << std::endl;
}

int main() {

    DoubleLinkedList myNode;
    //Node* newNode = new Node(2,7);
    std::shared_ptr<Node> newNode = std::make_shared<Node>(2, 7); // enter key number and data number
    std::shared_ptr<Node> newNode1 = std::make_shared<Node>(3, 9);// enter key number and data number

    newNode->key;
    newNode->data;
    myNode.addNodeToFront(newNode);


    newNode->key;
    newNode->data;
    myNode.addNodeAfter(2, newNode1); // enter the key number of existing node and then to add new key number and new data
    myNode.printList();

   

    system("pause");
    return 0;
}

The error I'm getting is:

unhandled exception thrown: read access violation. "this" was 0x8
To elaborate -
( Access violation reading location 0x0000000000000010. Unhandled exception thrown: read access violation).

So, the code would work when I use raw pointers. The only thing I can deduce from this is that there's a chain of shared pointers in the method of void DoubleLinkedList::addNodeAfter() nextNode = mypointer-> next;

or that shared pointers can't have nullptr assigned to it.

I'm clueless as to why this is happening.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
sampro7
  • 1
  • 1
  • 1
    you probably have a null pointer to a `Node`. e.g. `Node* node = nullptr; node->next` would probably produce a similar error. Have you tried using a debugger? – Alan Birtles Nov 30 '22 at 17:13
  • `mypointer-> next = n;` followed by `n -> previous = mypointer;` is a great recipe for [memory leak](https://stackoverflow.com/questions/22185896/what-is-the-cyclic-dependency-issue-with-shared-ptr). For the crash, just run your code in the debugger and look at the stack trace. In your question there is a lot of unnecessary code, so it's quite annoying to try copy-paste it and compile. – pptaszni Nov 30 '22 at 17:15
  • Also note that you likely have cycles in your shared pointers so they won't automatically destroy themseleves – Alan Birtles Nov 30 '22 at 17:18
  • yes i'm using visual studio 2019 local windows debugger. as i start it the window closes and exeption is thrown. and i dont really have a nullptr to node. because when i run the program two if statements work where no node exist & node already exists when it gets to the else statement this is the next line too be executed "nextNode = mypointer-> next;" – sampro7 Nov 30 '22 at 17:22
  • Have Visual Studio break on access violation exceptions. It should have been enabled by default. The setting should be in the Win32 part: [https://learn.microsoft.com/en-us/visualstudio/debugger/managing-exceptions-with-the-debugger?view=vs-2019](https://learn.microsoft.com/en-us/visualstudio/debugger/managing-exceptions-with-the-debugger?view=vs-2019) – drescherjm Nov 30 '22 at 17:28
  • OT: In `checkNodeExsits(int k)` no need to continue to iterate after `temp = mypointer;` you should put a `break;` after this line in the {} scope. This won't fix the bug but make the code more efficient. – drescherjm Nov 30 '22 at 17:39
  • @pptaszni i have chacked the stack and it says it is unable to read the memory of key and data in my Node.h – sampro7 Nov 30 '22 at 17:40
  • Where and how are these key and data corrupted? What function was executing at the time? Look up the callstack. I still bet the Node is a nullpointer. You should edit your question and add this important information when you find out. – drescherjm Nov 30 '22 at 17:42
  • 1
    Mmm yeah, don't describe what you read in your console, rather copy-paste the stacktrace if you have problems analyzing it. Normally it shows you the exact place in your code where you try to use the `nullptr`. Crash caused by memory violation is just the end effect. – pptaszni Nov 30 '22 at 17:45
  • @pptaszni yes thats what it showing to me on the stack call. key and data are assigned to 0 is that the issue ? – sampro7 Nov 30 '22 at 18:09
  • ***is that the issue ?*** That may be the issue but it does not tell anymore than you already know. You need to look further back on the callstack to see how the `Node*` became a nullptr in the first place. What functions were executing? Look at the variables in those functions. You can on the "StackFrame" combo box on the debug toolbar switch the callstack up one item and look at the variables in the previous function on the callstack and see what was going on. If that does not tell you more go up another level. – drescherjm Nov 30 '22 at 18:53
  • 1
    *So the code would work when i use raw pointers.* maybe. Maybe not. Could be the result of the bug merely wasn't easily observable. – user4581301 Nov 30 '22 at 19:58

1 Answers1

4

Let's take a walk through addNodeAfter

mypointer = checkNodeExsits(k);

We've checked to make sure value k exists in the list and gotten a pointer to it if it does. If it doesn't, we get a null pointer.

    if (mypointer == nullptr) {

Tests whether or not k was found. Let's assume it was, mypointer isn't null, and we enter the else

    else {
        if (checkNodeExsits(n -> key) != nullptr) {

Here we check to make sure the node we're inserting isn't a duplicate. Again let's take the else branch

        else {
               nextNode = mypointer-> next;

Should be safe, right? We know that mypointer was not null because we tested it earlier. But when we look... Holy smurf! The program crashed! It was null. How did it become null?

The answer lies in another question: Where did mypointer come from? It's not defined within this function, so it has wider scope. Turns out it is a DoubleLinkedList member variable. Is someone else messing with it? We don't have multiple threads, so it must be another function that was called by addNodeAfter.

That would have to be checkNodeExsits, so let's take a look at it.

std::shared_ptr<Node> DoubleLinkedList::checkNodeExsits(int k)
{
     temp = nullptr;
     mypointer = head; // well lookie here!

    while (mypointer != nullptr) {
        if (mypointer -> key == k) {
            temp = mypointer;
        }

        mypointer = mypointer-> next; // and here!

    }

    return temp;
}

We can see that if the inserted node's value does not exist, thus putting us in the else case back in addNodeAfter where we're going to insert the new node, mypointer can only be null!

The overly broad scope of mypointer turns the member variable into a boobytrap. It needs to be a variable local to checkNodeExsits and addNodeAfter to prevent these functions from trashing the state of the functions that use them. This leads to questioning whether mypointer should be a member variable anywhere it is found. That leads to wondering about temp, nextnode and prevnode. They all sound like temporary holders of local state information. Likely the only DoubleLinkedList member that should be a member is head.

To fix: Remove the definition of mypointer from the class and resolve the compiler errors that result. If a use of mypointer can be easily replaced with a local variable, do so. For any that remain and are needing of longer-term storage for resuse, you'll have to get a bit more creative. Make the call on how long-lived and how wide a scope is necessary to get the behaviour you want and then give this new variable an appropriately descriptive name to help you remember the circumstances in which it should be used.

I recommend repeating this process for the other member variables except for head where the necessary scope and lifetime is obvious.

General rule of thumb: Keep the scope of variables as tight as possible. Only widen the scope of a variable if you absolutely need to reuse its state later, and if you find you must be very careful with it to ensure you don't cause unintended side-effects.

user4581301
  • 33,082
  • 7
  • 33
  • 54
  • That's it, thanks alot! Now i've learned that wider scope pointers are frickin dangerous! – sampro7 Nov 30 '22 at 22:28
  • @sampro7 Too wide a scope on anything is risky. Occasionally we'll see a question about a `for` loop not working that turns out to have failed because the index variable had been used in a previous loop and already been incremented to the end or beyond. – user4581301 Nov 30 '22 at 23:00