-4

I'm basically trying to reverse the stack by passing it to a function. However, the program just crashes when I run it and I can find no logical errors as I have even overloaded the assignment operator to deal with pointer data members.

#include <iostream>

using namespace std;

/***************************************************************************/

class AStack {
public:
    AStack();
    AStack(int);
    ~AStack();
    AStack operator = (AStack s);
    void push(int);
    int pop();
    int top();
    bool isEmpty();
    void flush();

private:
    int capacity;
    int* a;
    int index = -1; // Index of the top most element
};

AStack::AStack() {
    a = new int[25];
    capacity = 25;
}

AStack::AStack(int size) {
    a = new int[size];
    capacity = size;
}

AStack::~AStack() {
    delete[] a;
}

AStack AStack::operator = (AStack s) {
    capacity = s.capacity;
    int* a = new int[capacity];
    for (int i = 0; i < capacity; i++) {
        a[i] = s.a[i];
    }
    index = s.index;
    return *this;
}

void AStack::push(int x) {
    if (index == capacity - 1) {
        cout << "\n\nThe stack is full. Couldn't insert " << x << "\n\n";
        return;
    }
    a[++index] = x;
}

int AStack::pop() {
    if (index == -1) {
        cout << "\n\nNo elements to pop\n\n";
        return -1;
    }
    return a[index--];
}

int AStack::top() {
    if (index == -1) {
        cout << "\n\nNo elements in the Stack\n\n";
        return -1;
    }
    return a[index];
}

bool AStack::isEmpty() {
    return (index == -1);
}

void AStack::flush() {
    if (index == -1) {
        cout << "\n\nNo elements in the Stack to flush\n\n";
        return;
    }
    cout << "\n\nFlushing the Stack:  ";
    while (index != -1) {
        cout << a[index--] << "  ";
    }
    cout << endl << endl;
}

/***************************************************************************/

void reverseStack(AStack& s1) {
    AStack s2;
    while (!s1.isEmpty()) {
        s2.push(s1.pop());
    }
    s1 = s2;
}

/***************************************************************************/

int main() {

    AStack s1;
    s1.push(1);
    s1.push(2);
    s1.push(3);
    s1.push(4);
    s1.push(5);
    reverseStack(s1);
    cout << "\n\nFlushing s1:\n";
    s1.flush();
    system("pause");
    return 0;
}
B-Mac
  • 11
  • 1
  • 6
  • You need to implement a proper copy constructor. See [The Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). – R Sahu Mar 11 '15 at 02:06
  • 1
    Crashes how? Where? – Scott Hunter Mar 11 '15 at 02:06
  • Hmmm...while we are at it, what is the third one in the rule of three besides Copy Constructor and the Overloaded Assignment Operator? I know the name, but what does it do? – B-Mac Mar 11 '15 at 02:19
  • @ScottHunter: Try running it buddy. – B-Mac Mar 11 '15 at 02:20
  • After writing a correct copy-constructor, implement copy-and-swap to make the assignment operator work. – M.M Mar 11 '15 at 02:31
  • @B-Mac as explained in R Sahu's link, the "three" are copy-constructor, copy-assignment operator, and destructor. The "five" are those two plus move-constructor and move-assignment operator. If you use copy-and-swap then the same function can fulfil both assignment operators. – M.M Mar 11 '15 at 02:32
  • @B-Mac Your assignment operator that you did write is also unorthodoxed. It should return a reference to the current object, not a new object. Returning a new object invoked that missing copy constructor, in addition to passing the argument by value initiated the copy constructor, so your assignment operator, as-is, is broken in at least 3 ways. – PaulMcKenzie Mar 11 '15 at 02:51

1 Answers1

1

You are not providing a copy-constructor, and your assignment operator takes the argument by value. The statement s1 = s2 creates a copy of s2 by calling the implicitly defined copy constructor that copies the pointer, then assigns to s1. At the end of the expression the copy is destroyed, calling delete [] on the pointer. At the end of the function the destructor for s2 runs and attempts to delete [] the same pointer again.

You need to provide a copy constructor that does the right thing.

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489