2

I've a very simple class. It has a pointer member variable. I want to overload the + operator. But, it doesn't work! Here is my code:

#include <iostream>
using namespace std;


template <typename T> 
struct Node{
    Node *next;
    T data;
};

template <typename T> 
class stack{
protected:
    Node <T> *head;

public:
    stack();
    ~stack();
    bool push(T);
    stack <T> operator+ (stack <T>);

};

template <typename T> 
stack<T>::stack(){
    head = NULL;
}

template <typename T>  
stack <T>::~stack(){
    Node <T> *temp;

    while(head){
        temp = head;
        head = head->next;
        delete temp;
    }
}

template <typename T> 
bool stack <T>::push(T data){
    Node <T> *newElem = new Node <T>;
    if (!newElem) return false;

    newElem->next = head;
    newElem->data = data;
    head = newElem;
    return true;
}

template <typename T> 
stack <T> stack<T>::operator+ (stack <T> stack1){
    stack <T> result;
    Node <T> *temp = head;

    while (temp) {
        result.push(temp->data);
        temp = temp->next;
    }

    temp = stack1.head;
    while (temp) {
        result.push(temp->data);
        temp = temp->next;
    }

    return result;
}

int main(){
    stack <int> myStack1, myStack2, myStack3;

    myStack1.push (1);
    myStack1.push (2);

    myStack2.push (3);
    myStack2.push (4);

    myStack3 = myStack1 + myStack2; //  here at runtime myStack3 is not the result of myStack1 + myStack2. 
    return 0;
}

Could you please help me with this? Please note that this is just for practice.

Thank you very much.

Daniel Frey
  • 55,810
  • 13
  • 122
  • 180
Nejla
  • 35
  • 1
  • 5

2 Answers2

2

You declared something else in class declaration, you wrote += instead of + in the definition:

stack <T> operator+ (stack <T>);
                  ^

stack <T> stack<T>::operator+= (stack <T> stack1){
                            ^^
masoud
  • 55,379
  • 16
  • 141
  • 208
2

Your problem is that you are creating a copy of the right-hand side of the operator. The signature should be:

template <typename T> 
stack <T> stack<T>::operator+ (const stack <T>& stack1) { ... }

so that you are only referencing stack1, not copying it.

This will fix the immediate problem you have, but long term, you also want to implement a correct copy-constructor as all copies of a stack will currently corrupt the memory as they link the same elements and the destrutor will delete them, the other stack then still references the deleted elements.

This is why you see "result" having the expected value, the copy called stack1 has not yet been deleted. But when the operator+ returns, it deletes stack1 and therefore all elements of stack1 are deleted. Not all pointers that were copied to the result that refer to elements from stack1 are invalid. This is why stack3 appears broken after the call.

See also the rule of three, read the "Managing resources" section in the answer.


Update: This is not enough as you also assign the result of operator+, again the rule of three applies as you don't have a correct assignment operator as well. You probably want to add these to your class:

template <typename T> 
stack<T>::stack(const stack<T>& src)
{
    // this will reverse the src, I'll leave it to you to fix the order!
    head = NULL;
    Node<T>* tmp = src->head;
    while( tmp ) {
        push( tmp->data );
        tmp = tmp->next;
    }
}

template <typename T> 
stack<T>& stack<T>::operator=( stack<T> src ) // copy the argument is OK here!
{
    stack old;
    old->head = head;
    head = src->head;
    src->head = NULL;
}

Since you are learning, this above is a good starting point. You now have to

  • Implement the copy-constructor in a way that it does not reverse the order of elements
  • Understand how exactly the assignment operator works. I used an unusual implementation, but once you understand it, you learned a lot about object lifetime :)
Community
  • 1
  • 1
Daniel Frey
  • 55,810
  • 13
  • 122
  • 180
  • It didn't solve the problem Daniel. myStack3 still shows irrelevant data. It seems it points to somewhere else. – Nejla Oct 12 '13 at 08:47