views:

454

answers:

2
+4  Q: 

AVL TREE in c++

I have a problem with this very simple block of code. please give me your advice . (My this problem is solved, and in solving this problem the person having id stakx really helped me, the only problem was that i was using stack< treeNode >, when i saw the push method of the stack carefully, there is a copying process when i write head->object=number, so finally i made the stack of pointers, like this stack< treeNode* > and it really solved the problem , i have no problem now , i am very very thankful to the person stakx.)

before the code you need to suppse the following tree

alt text
as you can see in the picture that root is 8 and stack have two nodes i.e 6 and 4. i pass this stack and root node to the following code

void Avltree::attachwithtree(treeNode* tree, Stack<treeNode>&s)
{
if(!s.isempty())
{
treeNode *stacknode;
stacknode=s.pop();
cout<<"\ninside the attachwithtree function, stack node is "<<stacknode->data;
stacknode->right=tree;//attaching the passed node to the right of popped node
root=stacknode;//setting the root to stack node which is the private data member of class
updatebalance(root);//this function is ok, it does not create problem
while(!s.isempty())
{
cout<<"\nstack is still not empty";
stacknode=s.pop();
cout<<"\nright side of "<<root->data<<" is "<<(root->right)->data;
//the below three lines causing the problem i don't know why,
root=stacknode;
treeNode* temp;
temp=root->right;
cout<<"\n\n\nthe right side of "<<temp->data<<" is now "<<(temp->right)->data;
updatebalance(root);
}

the ouptput of this function is given by
alt text


here is the code of the pop method of the stack that i am using

template <class t>
t * Stack<t>::pop()
{
if(topelement!=NULL)
{
t* num;
current=topelement;
num=&(current->object);
topelement=topelement->preptr;
current=topelement;
return(num);
}
else
{
head=NULL;
}
}


here is the code of push method of the stack

template <class t>
void Stack<t>::push(t &number)
{
Node<t>* newNode=new Node<t>;
if(head==NULL)
{
head=newNode;
topelement=newNode;
current=newNode;
head->object=number;
head->preptr=NULL;
}
else
{
topelement=newNode;
newNode->preptr=current;
current=topelement;
newNode->object=number;
}
}
+5  A: 

Original answer:

Could it be that the node 4 on the stack has a different node 6 to its right (the one with node 7 to its right) than the node 6 (with the node 8 on its right) you're working on? You could compare their addresses to make sure you haven't got two different copies of node 6 around.

Elaboration on the above argument:

Let's look at your method's signature:

void Avltree::attachwithtree(treeNode* tree, Stack<treeNode>&s)

s is defined as a reference to a Stack<treeNode>.

Could it be that it should be a Stack<treeNode*>?

Depending on your treeNode class, it's possible that when you push X on this stack, you actually end up with a copy of X and not X itself. Similarly, when you pop from the stack, you might not actually get the item you pushed, but an identical-looking copy of it!?

This would mean that, at the time when you push node 6 on the stack, its right child is node 7. But you have pushed a new, identical node on the stack. Even if you pop this element from the stack and change it, you only change a copy and leave the original tree node as it was before.

Therefore, you'd operate on different copies of node 6. First, you pop a copy of it from the stack, and append a tree as its right child. Checking this will give the right result.

Then, you pop a copy of node 4 from the stack. It's right child is node 6, as expected, BUT not the one you just modified but the original! Therefore you get 7 on the right side of node 6.

Demonstrating the difference between pass-by-value and pass-by-reference:

OK, here's something you need to understand when working with pointers or references. It basically shows the difference between passing a parameter by value (a copy will be created) or passing it by reference (no copy will be created).

Study it carefully and then see how it applies to your problem.

#include <iostream>

class someObject
{
private:
    int _value;
public:
    someObject(int value) : _value(value) { }

    int getValue()
    {
        return _value;
    }
};

void someFunction(someObject objCopy, someObject* objPtr)
{
    std::cout << "objCopy.getValue() -> " << objCopy.getValue() << std::endl;
    std::cout << "objPtr->getValue() -> " << objPtr->getValue() << std::endl;
    if ( &objCopy != objPtr )
    {
        std::cout << "objCopy is not actually *objPtr but a copy of it." << std::endl;
    }
    else
    {
        std::cout << "objCopy and *objPtr are one and the same object." << std::endl;
    }
}


int main()
{
    someObject X(17);
    someFunction(X, &X);

    return 0;
}

Hint: Yes, in your pop method, you work with pointers, but most likely with a pointer to a copy of the object originally pushed on the stack.

stakx
no this is not the case i have tested now the right node of 4 is 6 i have mentioned in the code that the three lines are causing the problem, before these three lines the right node of 6 is 8 but after these three lines the right node of 6 becomes 7.
Zia ur Rahman
OK. Still, please bear with me once again and re-read my answer. I've expanded it in the same direction I was going before.
stakx
I agree with stakx. Are you sure that the node 6 is not duplicated for some reason? Because from your code it is almost absolutely clear that root->right before those three lines points to different address than stacknode->right at that point. It's node 6, but a different one. Try to add the following printout just before the comment "the below three lines causing the problem i don't know why":cout<<
Dmitry Perets
theoratically you are looking right , but not actually because you can see in the pop method i am returning the pointer to the top element not the copy , similarly i have push method where i am pushing addresses of nodes not the copies. your answer is still not helpful.
Zia ur Rahman
still not clear , Ok if you are right then suggest me how should i write my pop method and push method , i am pushing addresses of nodes in the push and returning pointer to those addresses from the pop method how would you write these two method so that the above problem can be solved.
Zia ur Rahman
i am posting the code of push method also.
Zia ur Rahman
AT last your answer helped me, yes there is a copying process in the push method if we see the code carefully,the problem is solved now , and how this is solved , i made a stack of pointers stack<treeNode*> instead of stack<treeNode> , and the problem is completly solved , so thank you once again , i am very happy now, because i could not find the solution to this problem since last 4 days and finally i got it. thank you thank you .
Zia ur Rahman
It's not so much that the `push` and `pop` methods are wrong (though I didn't look at them in detail), but what *kind* of objects you put on the stack. Currently, your stack has `treeNode` s on it. If you want to be able to modify a node in the original tree, you must instead push a *reference/pointer* to it on your stack (a `treeNode*`; therefore your stack should be a `Stack<treeNode*>`). With a `Stack<treeNode>`, everytime `push` or `pop` operation creates a new `treeNode` copy. You can then modify a popped copy as much as you want, the original tree node will remain unmodified.
stakx
Ah, OK, saw your comment too late. I'm glad I could help.
stakx
ya sir! i am happy now, and very thankful to you
Zia ur Rahman
+1  A: 

Is that your own Stack class? A quick look at the STL tells me that pop() has return type void.

It could be that stakx is onto something here. If your pop() function returns a copy of the top element rather than a reference to it, the changes you make will only apply to the copy. Do you explicitly add the copy back into the tree anywhere after modifying it?

UncleO
i am posting the code of pop method , look at it i am returning pointer to the top element from the pop method
Zia ur Rahman