Resize in dynamic pointer transforms dimension into garbage

3

I was responding to a std :: stack implementation problem. It was easy, but I could not use my first idea: std :: vector (it replaced forever the dynamic pointers). My code is:

template <class T> class stack
{
    T* data;
    unsigned size_;

    public:
    stack() : size_(0){}
    stack(T initializer, unsigned times)
    {
        size_ = times;
        data = new T[times];
        for(int i = 0; i < times; i++) data[i] = initializer;
    }

    void push(T data_)
    {
        size_++;
        data = new T[size_];
        data[size_-1] = data_;
    }
    void pop()
    {
        size_--;
        data = new T[size_];
    }
    T top() const
    {
        return data[size_-1];
    }
    void clear()
    {
        for(unsigned i = 0; i < size_; i++)
        {
            data[i] = 0;
        }
    }
    bool empty() const
    {
        bool ret = true;
        for(unsigned i = 0; i < size_; i++)
        {
            if(data[i] != 0)
            {
                ret = false;
                break;
            }
        }
        return ret;
    }
    unsigned size() const
    {
        return size_;
    }
    void resize(unsigned newsize)
    {
        size_ = newsize;
        data = new T[size_];
    }
};

I decided to test with an integer; and it worked. However, I did one of size 2 and decided to test the pop method.

int main()
{
    stack<unsigned> A;
    A.push(10);
    A.push(2);
    std::cout << A.top() << "\n";
    A.pop();
    std::cout << A.top();
}

There, the top method returns garbage. What am I doing wrong?

    
asked by anonymous 20.04.2014 / 21:41

2 answers

6

The first point we can improve is your memory allocation. Allocating and deallocating each operation works, but it is not efficient. A good solution is to allocate a larger block than was initially necessary, and make new allocations only when it is full, just as it is in std::vector . For this we need to add a capacidade field to the stack:

T *m_data;
std::size_t m_size;
std::size_t m_capacity;

Initialize everything with null and zeros. The push and pop can look like this:

void push(const T &value) {
  if (m_capacity == 0) {
    m_capacity = 1; //Capacidade mínima
    m_data = new T[1];
  }
  else if (m_size == m_capacity) {
    T *old = m_data; //Guarda ponteiro
    m_capacity *= 2; //Dobra capacidade
    m_data = new T[m_capacity]; //Aloca novo array
    std::copy(old, old+m_size_, m_data); //Copia dados
    delete[] old; //Libera array antigo
  }
  m_data[m_size] = value; //Guarda novo valor
  ++ m_size; //Incrementa tamanho
}

void pop() {
  --m_size; //Basta decrementar o tamanho
}

You do not need to deallocate in pop , because when you have a new push it will take advantage of the old array. But you can create a trim function to eliminate unnecessary space.

void trim(const T &value) {
  if (m_capacity == m_size) {
    return;
  }
  T *old = m_data; //Guarda ponteiro
  if (m_size == 0) {
    m_data = NULL; //Para tamanho zero o array é nulo
  }
  else {
    m_data = new T[m_size]; //Aloca novo array
    std::copy(old, old+m_size_, m_data); //Copia dados
  }
  m_capacity = m_size; //Guarda novo valor de capacidade
  delete[] old; //Libera array antigo
}

Of course, you should not forget to delete the array in the destructor. A copy constructor and assignment operator are also required, and if you are already with c ++ 11 you can do the builder and assignment operator too.

The other point is its functions clear and empty . They put the value zero on the stack, and check the amount of that value. That way you can not differentiate an empty stack from a full of zeros. In clear you should just zero m_size and in empty just check if it is zero.

void clear() {
  m_size = 0;
}

bool empty() {
  return m_size == 0;
}

Now it would only be enough to add elements access functions by index that would practically have a reimplementation of std::vector for simple types. To be complete it would be necessary to allocate the memory of the array in a raw way and construct and destroy the objects manually ( placement contructor and explicit call to the destructor) so that objects are not created before the time nor do they continue to exist after necessary.

    
21.04.2014 / 15:34
0

With the help of @ C.E.Gesser, I got it (I forgot how to do the resize correctly), I got the code. It looks like the following:

#include <iostream>
#include <algorithm>
template <class T> class stack
{
    T* data;
    unsigned size_;

    public:
    stack() : size_(0){}
    stack(T initializer, unsigned times)
    {
        size_ = times;
        data = new T[times];
        for(int i = 0; i < times; i++) data[i] = initializer;
    }

    void push(T data_)
    {
        T* temp = new T[size_ + 1];
        std::copy(data, data+size_, temp);
        size_++;
        data = new T[size_];
        std::copy(temp, temp+size_, data);
        data[size_-1] = data_;
    }
    void pop()
    {
        T* temp = new T[size_-1];
        std::copy(data, data+(size_-1), temp);
        size_--;
        data = new T[size_];
        std::copy(temp, temp+size_, data);
    }
    T top() const
    {
        return data[size_-1];
    }
    void clear()
    {
        for(unsigned i = 0; i < size_; i++)
        {
            data[i] = 0;
        }
    }
    bool empty() const
    {
        bool ret = true;
        for(unsigned i = 0; i < size_; i++)
        {
            if(data[i] != 0)
            {
                ret = false;
                break;
            }
        }
        return ret;
    }
    unsigned size() const
    {
        return size_;
    }
    void resize(unsigned newsize)
    {
        size_ = newsize;
        data = new T[size_];
    }
};

int main()
{
    stack<unsigned> A;
    A.push(10);
    A.push(2);
    std::cout << A.top() << "\n";
    A.pop();
    std::cout << A.top();
}
    
21.04.2014 / 00:35