-3

I am constructing an AVL tree program. I got stuck in a rather easy situation, but difficult to understand what is wrong. The reason I think it is the program's fault and not mine is because I have the exact same class function right before it with "left" and "right" swapped and it works just fine...

As you can see, the function returns the temproot pointer which is equal to temp2 if root==temp. The funny thing is, that although when I test-print the value of temproot JUST before it returns it (that in my example is 15), the actual value that is returned is STILL 20 (the previous value of temproot). I triple checked everything. It doesn't seem to return the newly acquired value... What may be the problem?

To be more specific, the exact code is this:

//structure
struct avlnode
{
    int data;
    avlnode * left;
    avlnode * right;
}* root;

//class function
avlnode * Tree::RL_rotation (avlnode * temp)
{
    avlnode * temproot = temp;
    avlnode * temp1= new avlnode;
    temp1=temp->right;
    avlnode * temp2= new avlnode;
    temp2=temp1->left;

    temp1->left=temp2->right;
    temp2->right=temp1;
    temp->right=temp2;

    temp->right=temp2->left;
    temp2->left=temp;

    if (root==temp)
    {
        root=temp2;
        temproot=temp2;
    }
    cout << "temproot= " << temproot->data << endl;
    return temproot;
}
Mogsdad
  • 44,709
  • 21
  • 151
  • 275
Manolis Grifoman
  • 15
  • 9
  • If what you saying is true, it might be a buffer overrun (memory corruption) issue. – SergeyA Oct 13 '15 at 13:42
  • How are you storing the return from the function? – NathanOliver Oct 13 '15 at 13:43
  • 1
    I don't believe you. How did you "check" those things? Does `temproot=temp2;` really gets executed? – Karoly Horvath Oct 13 '15 at 13:44
  • 6
    The "irrelevant" code might not be that irrelevant after all... – SingerOfTheFall Oct 13 '15 at 13:44
  • I even tried to change the variables but nothing happened. Of course, temp2 isn't 15, but it points to a structure node with an int variable named "data" of the value of 15. How can I solve the beffer overrun? – Manolis Grifoman Oct 13 '15 at 13:45
  • I checked it by putting cout << temproot->data << endl; right before it returns it... – Manolis Grifoman Oct 13 '15 at 13:47
  • The irrelevant code is about other nodes. temp1 and temp2... – Manolis Grifoman Oct 13 '15 at 13:47
  • Are you sure you didn't mean to assign values? Do you know how to use pointers? Not enough info, sorry. – NineToeNerd Oct 13 '15 at 13:52
  • 1
    The problem is not in the posted code. – Karoly Horvath Oct 13 '15 at 13:52
  • I updated with the exact code that I thought was "irrelevant". The most funny fact of all is that I have the same exact function in this class right above this function, which has the same variables (but the left and right are reverse) and it runs completely fine... – Manolis Grifoman Oct 13 '15 at 14:02
  • 1
    You're allocating new nodes which you immediately leak. – molbdnilo Oct 13 '15 at 14:48
  • What do you mean by the "previous value" of `temproot`? The value of `temp->data` when entering the function? – molbdnilo Oct 13 '15 at 14:51
  • "temp" is assigned to "temproot". After that, in the "if" you can see that now "temp2" is assigned to "temproot". But "temproot" doesn't change... It still holds the value of "temp" and not "temp2" – Manolis Grifoman Oct 14 '15 at 14:54
  • I just found what I did wrong and indeed it was not in the code displayed. It was in the class function that called the RL_rotation function. It turns out that although the RL_rotation is returning something, I just wrote in the function that called the RL_rotation: RL_rotation(temp)... As if it were void. And the compiler didn't recognise the error. How is it possible? – Manolis Grifoman Oct 14 '15 at 15:02

1 Answers1

0

I don't see a problem in the functionality you are seeing.

Pointers work in different ways than your typical objects (values), and are pretty hard to explain without having drawing board at hand, but let me try.

When you "remember" the pointer to initially passed in node, you are remembering just that. Even if you assign initial pointer to different structure fields, "move it around", your "copy" will still point at the exact same element, since the actual memory doesn't move when you are assigning/reassigning pointers. And, because of that (in your case), the rotation you are doing, won't be reflected by the parent element, of the initially passed in root, since it would still be pointing to the same element that got assigned to temp2->left (as is temproot, since the actual memory weren't reassigned).

If you want to change (have access to) the location where the actual element is stored. As I assume is within your case, you need to pass in the root element of the rotation by reference (avlnode* Tree::RL_rotation (avlnode*& temp)). And by doing that, you will pass, to the function, not only the memory location of your node, around which you want to do the rotation, but the location of the location as well. Which would allow to change the root, after the rotation, which you weren't able to do before.

Note: Get rid of code like this: avlnode * temp1= new avlnode; temp1=temp->right;

You are creating a new memory location, which you immediately forget, and it never gets released (there's no garbage collector in native C++).

Instead, write like this (since you don't really need to create new node - you are only rearranging them): avlnode * temp1= temp->right;

Algirdas Preidžius
  • 1,769
  • 3
  • 14
  • 17