array based stack

[cppnoob25]

Can anyone spot my error here?
I'm doing this for practice and can't figure out how I'm changing the value of top here to 8.

#include <iostream>

using namespace std;

class Stack{
public:
    Stack(const int size);
    void push_back(int value);
    void print();

private:

int max;
int myarray[];
int top = -1;

};

Stack::Stack(const int size)
{
    max = size;
    myarray[max];
}

void Stack::push_back(int value)
{
    top++;
    if(top < max)
    {
        myarray[top] = value;
    }
    else{
        cout << "stack is full"<<endl;
        top--;
    }
}

void Stack::print()
{
    for(int i = 0; i <top;i++)
    {
        cout << "----------" << endl;
        cout << myarray[i] << endl;
        cout << "----------" << endl;
    }
}

int main()
{
    Stack object(10);
    object.push_back(8);
    object.print();
}

[MatthewRock]

Is this even valid C++? Line 22, for example. What should that mean?

You'd have to change it to either use constant size, specified at compilation time, or dynamic array(but then you'd probably prefer either unique_ptr'd it, or just std::vector).

If you're asking how that program(assuming it'd be valid) would work, it's quite simple. You keep track of size of stack. Its head is element in array on size-th place. If you have 0 elements, you start inserting at 0. If you have 10 elements, next one will go to 10th place.

And if you exceed maximum size, you report overflow, somehow.

However, if you're trying to learn from this code, dump it. It's quite bad. If you want to learn data structures, I recommend you this book: http://en.wikipedia.org/wiki/Introduction_to_Algorithms . It's good book to learn data structures, and algorithms related to it.

[andywestken]

Rather scarily the code compiles as-is with (the MinGW version of) GCC 4.8.1 :-(

(But it fails to compile with Visual Studio, with "error C2229: class 'Stack' has an illegal zero-sized array")

Line 22 is valid but rather pointless; why access the member of an array but then not use it? And if max is the size of the array, then you're trying to access the element just after the end of the array.

The line which concerns me more is line 14 -- the zero size array. Apparently an extension from C (but according to the C standard only the last member of a struct is allowed to be "flexible"?)

The output is:

----------
8
----------
----------
4201936
----------
----------
2359088
----------
----------
2359200
----------
----------
4199443
----------
----------
0
----------
----------
0
----------
----------
0
----------

Counting the output lines I see that there are 8 lines, so setting myarray[0] is trampling on top as myarray has been declared with a size of zero. This is not legal in standard C++.

Adding "-std=c++11" to my compiler options didn't stop the code from building with GCC as expected? :-(

But adding -Wpedantic ("Issue all the warnings demanded by strict ISO C and ISO C++; reject all programs that use forbidden extensions, ...") I get

.../test/main.cpp:14:13: warning: ISO C++ forbids zero-size array 'myarray' [-Wpedantic]

And adding -Wall -Wextra spots the pointless array index operator usage.

.../test/main.cpp:22:16: warning: statement has no effect [-Wunused-value]

#include <iostream>

using namespace std;

class Stack{
public:
    Stack(const int size);
    ~Stack(); // add destructor
    void push_back(int value);
    void print();

private:

int max;
//int myarray[];
int* myarray; // now a pointer
int top = -1;

};

Stack::Stack(const int size) : max(size), myarray(new int[size])
{
    //max = size;
    //myarray[max];
}

Stack::~Stack()
{
    delete [] myarray; // free memory
}

void Stack::push_back(int value)
{
    top++;
    if(top < max)
    {
        myarray[top] = value;
    }
    else{
        cout << "stack is full"<<endl;
        top--;
    }
}

void Stack::print()
{
    //for(int i = 0; i <top;i++)
    for(int i = 0; i <=top;i++) // need <= not <
    {
        cout << "----------" << endl;
        cout << myarray[i] << endl;
        cout << "----------" << endl;
    }
}

int main()
{
    Stack object(10);
    object.push_back(8);
    object.print();
}

The main change is to replace the zero size array with a pointer; C++ allow allows fixed sized arrays, so if you want to determine the size at runtime you need to use a pointer. Also fixed loop termination condition.

----------
8
----------

(Of course, in real life -- rather than an exercise -- you'd use std::vector rather than an array. And std::stack instead of a custom one...)

Andy

PS When using GCC for educational purposes, you should consider specifying the standard (e.g. -std=c++11) and crank up the error level (-Wall -Wextra -Wpedantic)

[cppnoob25]

Thanks for the help Andy. I was going for a dynamic array and somehow forgot to implement it after I started. Guess I need more practice with dynamic memory.

[andywestken]

In case anyone is interested...

What is the purpose of a zero length array in a struct? [duplicate]
http://stackoverflow.com/questions/11733981/what-is-the-purpose-of-a-zero-length-array-in-a-struct

... for why C programmers want a zero size array!

Andy