-1

I've been trying to do unit testing for my AVL Tree class and when I call the "depth()" function which calls a recursive "depth(...)" I get the following exception Code: Error Code: C0000005

This is my code for the depth function:

int Tree::depth()
{
    int currentDepth = -1;
    if (this->isEmpty() == false) {
        currentDepth = depth(this->root, currentDepth);
    }
    return currentDepth;
}

int Tree::depth(Node* node, int currentDepth)
{
    int nodeDepth = currentDepth;
    if (node->left != nullptr) { // <-------------- Error happens at this line
        nodeDepth = depth(node->left, currentDepth);
    }
    if (node->right != nullptr) {
        nodeDepth = depth(node->right, currentDepth);
    }
    return nodeDepth + 1;
}

I've looked up what can cause this error and most people say it is due to an uninitialized pointer but I've tried many ways of initializing the pointer to no avail. This is what I have now:

struct Node {
    public:
        TreeElement* element;
        Node* parent;
        Node* left;
        Node* right;

        Node(TreeElement* element, Node* parent) {
            this->element = element;
            this->parent = parent;
            this->left = nullptr; // <------------ This part here
            this->right = nullptr;
        }

        ~Node(){}


    };

When I debug the test it ends up lifting this exception: Read access violation

I'm trying to find out how to fix this error.

Here's my Unit test and code to add a node to the tree in case you need it

Test Method:

TEST_METHOD(WHEN_anElementIsAddedToATree_THEN_itHasCorrectDepth)
{
    // Arrange
    Tree tree;
    Word* element = new Word(ANY_WORD);

    // Act
    tree.add(element);

    // Assert
    int ANSWER = tree.depth();
    const int EXPECTED_ANSWER = 0;
    Assert::AreEqual(ANSWER, EXPECTED_ANSWER);
}

Add Function:

void Tree::add(TreeElement* element)
{
    if (this->isEmpty()) {
        Node node = Node(element, nullptr);
        this->root = &node;
    }
    else if (this->find(element) > -1){
        throw "Cet élément est déjà présent";
    }
    else {
        add(element, this->root);
    }
}

void Tree::add(TreeElement* element, Node* node)
{
    if (element->isBiggerThan(node->element)) {
        if (node->right != nullptr) {
            add(element, node->right);
        }
        else {
            node->right = &Node(element, node);
        }
    }
    else {
        if (node->left != nullptr) {
            add(element, node->left);
        }
        else {
            node->left = &Node(element, node);
        }
    }
}
SherylHohman
  • 14,460
  • 16
  • 79
  • 88
  • 2
    What happens if `node` is a `nullpointer`? – drescherjm Dec 11 '19 at 19:38
  • 1
    Psychic Debugger says: Incorrect copy constructor and copy assignment operators, conflicting with Destructor. – Mooing Duck Dec 11 '19 at 19:39
  • 1
    *but I've tried many ways of initializing the pointer to no avail* -- How do you know initialization is the cause of the error? What if your `Tree` just happens to mismanage things, and it just so happens you are sending invalid pointers to it? Bad coding, even with "proper initialization", can cause these issues. – PaulMcKenzie Dec 11 '19 at 19:41
  • `node` is almost certainly bogus, but we have too little information to tell you what made it bogus. Check for dancing gremlins on line 42. – user4581301 Dec 11 '19 at 19:43
  • If I recall, `0xCCCCCCCC` (from your screen shot) is uninitialized pointer in VS in Debug. – ChrisMM Dec 11 '19 at 19:44
  • 1
    Also, since you are using the debugger, *follow your code path slowly by single-stepping in the debugger*. There is a reason for the invalid pointer, and you just have to single-step through your code to find where/why your code is faulty. – PaulMcKenzie Dec 11 '19 at 19:47
  • 1
    0xCCCCCCCC = Uninitialized Stack Memory. [https://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations/127404#127404](https://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations/127404#127404) – drescherjm Dec 11 '19 at 19:47
  • @ChrisMM brings up a good point. When you see a highly unlikely number like CCCCCCCC in an error message, you should [look it up to see if it means something](https://en.wikipedia.org/wiki/Magic_number_(programming)#Magic_debug_values). Keep an eye out for BEEF, DEAD, F00D, and repetition. – user4581301 Dec 11 '19 at 19:48
  • 2
    `this->root = &node;` ... `node->right = &Node(element, node);` is a bug that you repeat often. You can't store the address of a temporary or local variable that goes out of scope. – drescherjm Dec 11 '19 at 19:51
  • Welcome to SO. Instead of editing your post to include an answer, you should `upvote` and/or `accept` answers & comments you find useful. This allows others to view the question, independent from any 1 particular answer. They can easily see if Q is relevant to their own situation. Scanning the list of answers, comments, and votes, visitors can quickly find what works for their situation. Keeping the original question up, & separating Q's from A's makes the site efficient, clear, and highly useful to future visitors. Our gratitude, reward, and feedback are in giving and receiving votes. :-) – SherylHohman Dec 17 '19 at 05:31

1 Answers1

0
void Tree::add(TreeElement* element)
{
    if (this->isEmpty()) {
        Node node = Node(element, nullptr);
        this->root = &node;
    }

You set this->root to point to node. But node is local to this if block. As soon as the if block exits, node no longer exits. So this->root is pointing to an object that no longer exists.

It is your responsibility to ensure that an object lives longer than any pointer to it that might be dereferenced. The easiest way to handle this is not to use "dumb pointers" but to instead use smart pointers like std::unique_ptr<Node>.

David Schwartz
  • 173,634
  • 17
  • 200
  • 267