handling arrays in class declaration

Question

I just wrote a simplified implementation of the stack data structure in a class, but the handling of an integer array is behaving in a way that I can't seem to understand.

The same snippet of code as in push() will give the behavior I expect, but in this program assigning a value at a certain array position will assign the value to the index variable>

 #include <iostream>
using namespace std;

class stack
{
public:
    stack(int size)
    {
        ar_size = size - 1;
        array[ar_size];
        index = 0;
    }
    void push(int value)
    {
        cout << "index; " << index << endl; //will output 0
        cout << "value: " << value << endl; //will output 8
        array[index++] = value;
        cout << "index; " << index << endl; //will output 8
        cout << "value: " << value << endl; //will output 8
        cout << "array: " << array[index] << endl; //will output what seems to be a memory address
    }
    int pop()
    {
        cout << "index; " << index << endl; //will output 8
        return array[index--];
    }
private:
    int ar_size;
    int array[];
    int index;
};
int main()
{
    stack tower(64);
    tower.push(8);
    int r = tower.pop();
    cout << "pop: " << r << endl; //will output what seemed to be a memory address

    return 0;
}

What is this statement actually supposed to do: `array[ar_size];`? I have a feeling it doesn't do what you think it does. — πάντα ῥεῖ, Apr 18 '15 at 09:47
Enable compiler warnings + read and understand warnings = PROFIT ! — Paul R, Apr 18 '15 at 09:53
@πάντα ῥεῖ I see your point, I had got an error by declaring the array size in the private definition and ended up making this mistake. so, how and where should I try to declare the array size? — maja, Apr 18 '15 at 09:53
@maja Best is to use a `std::vector`, and initialize it with `ar_size` in the constructor member initializer list: `std::vector array;` `stack(int size) : array(size) { // ...` — πάντα ῥεῖ, Apr 18 '15 at 09:59
@Hiroaki I'm not sure what result I should expect, if I try this in the constructor the program chrashes, if I try it in the private section I get the same error as trying to set `array[]` with a size: invalid use of non-static data member stack::ar_size`. — maja, Apr 18 '15 at 10:01
@Paul yes, in this case it pointed out that `array[ar_size];` was not having any effect (and thus no error was raised). it looks like a very useful kind of warning. — maja, Apr 18 '15 at 10:30
@maja: good - you should always take notice of warnings - this can save you an awful lot of debugging time later. — Paul R, Apr 18 '15 at 10:50

score 3 · Accepted Answer · answered Apr 18 '15 at 10:03

Here is the corrected code of your example:

#include <iostream>

class stack
{
public:
   stack(int size)
   {
      ar_size = size - 1;
      array = new int[size];
      index = 0;
   }
   void push(int value)
   {
      array[index++] = value;
   }
   int pop()
   {
      return array[--index];
   }
   ~stack()
   {
      delete array;
   }
private:
   int ar_size;
   int *array;
   int index;
};

int main()
{
   stack tower(64);
   tower.push(8);
   int r = tower.pop();
   std::cout << "pop: " << r << std::endl; //Will output 8 :)

   return 0;
}

There were several issues with it.

As pointed out in the comments array[ar_size]; in your constructor did not do what you wanted it to. array[ar_size]; accesses the array at the given index, it does not allocate the array for you. I've fixed the problem so that the array is now allocated via new and deleted when the stack is destroyed.
return array[index--]; was not right as well. You need to lower the index before accessing the element. return array[--index]; is now right.
You're missing a BUNCH of checks so that your stack does not cause a segfault or any other undefined behaviour. You need to check if you can still push values or if you can pop values and so on.

I hope it clears things up a bit.

the one thing I could not have spotted myself is the fact that I need to use the heap for handling an array between the function of a class, I'll change the title according to that.. (it's still too early to assess segfaults for me). — maja, Apr 18 '15 at 10:19
You are not required to use the heap. But you can not dynamically allocate memory on the stack in that way. If you wanted there are [some ways](http://stackoverflow.com/questions/6335023/c-how-to-allocate-memory-dynamically-on-stack) to do it but it's a lot easier this way. — Majster, Apr 18 '15 at 10:23
I'll have to read for a while with that, but the heap seems good enough for now. thanks. — maja, Apr 18 '15 at 10:29

Gaurav Sehgal · Answer 2 · 2015-04-18T10:11:02.640

You could use dynamic memory allocation.Something like this

private:
int ar_size;
int *array;//pointer to array
int index;

and then in the constructor

stack(int size)
{
    ar_size = size - 1;
    array=new int[ar_size];
    index = 0;
}

Since this is dynamic memory allocation make sure to free the allocated memory.You can have a destructor

~stack()
{
  delete[] array;
}

Another point is after you push an element,you increase the index by 1.So now index does point to the next insertion point in the stack.So if you do a pop operation it will remove an element from index location but there is no element there yet.So you can change your pop function to

int pop()
{
    cout << "index; " << index << endl; //will output 8
    return array[--index];//now index will point to the top element
}

You're missing a `delete` somewhere. This will cause a memory leak. A destructor which would clean up would be nice. — Majster, Apr 18 '15 at 10:05

score 1 · Answer 3 · answered Apr 18 '15 at 10:00

1

I think you want array = new int[ar_size]; instead of array[ar_size];. You'll need to make a destructor that does delete [] array; as well then.

answered Apr 18 '15 at 10:00

Jordi Vermeulen

1,168
1
10
17

handling arrays in class declaration

3 Answers3