How to use memcpy correctly with different types of arrays

Question

I have a templated class named Stack that has a private method resize.

Whenever I use the int or double template version of this class, it works as desired. But, whenever I use the float or string versions of the template, it crashes. I believe that it's a problem with my memcpy function call. How am I supposed to use it correctly?

template <class Type>

void Stack<Type>::resize(int capacity) {
    if(capacity >= MAX_SIZE)
        capacity = MAX_SIZE;

    Type* copy = new Type[capacity];

    for (int i = 0; i < N; i++) {
        copy[i] = s[i];
    }

    s = new Type[capacity];

    memcpy(s, copy, sizeof(Type) * capacity);

    size = capacity;

    delete copy;
}

s is a heap allocated member variable array of type Type.

Answer 1

First of all, it is incorrect to use memcpy to copy non-POD types. Just use a for loop, or std::copy.

Second, you are doing more work than necessary (and you have a memory leak).

void Stack<Type>::resize(int capacity) {
    if(capacity >= MAX_SIZE)
        capacity = MAX_SIZE;

    Type* copy = new Type[capacity];

    for (int i = 0; i < N; i++) {
        copy[i] = s[i];
    }

Up to this point, you're okay. You've allocated a new array, and assigned over the elements from the old array. I'm assuming N is the number of valid elements.

s = new Type[capacity];

Assuming that s previously pointed to an allocated array, this is a memory leak. First, you need to delete the previous data.

delete [] s;

Then, you don't need to allocate another array. Use the one you just allocated.

s = copy;

All together, the function now looks like this:

template <class Type>
void Stack<Type>::resize(int capacity) {
    if(capacity >= MAX_SIZE)
        capacity = MAX_SIZE;

    Type* copy = new Type[capacity];

    for (int i = 0; i < N; i++) {
        copy[i] = s[i];
    }

    delete [] s;
    s = copy;
    size = capacity;
}

If there are still problems, some other part of your code is broken.

Answer 2

template <class Type>
void Stack<Type>::resize(int capacity)
{
    if (capacity > MAX_SIZE)
        capacity = MAX_SIZE;
    else if (capacity < 0)
        capacity = 0;

    if (capacity == size)
        return;

    Type* new_s = new Type[capacity];
    std::copy(s, s + std::min(size, capacity), new_s);   
    delete[] s;
    s = new_s;
    size = capacity;

    if (N >= size)
        N = size-1;
}

Or, you could simply make your Stack use a std::vector instead of a raw array, and then you do not have to worry about these details anymore:

template <class Type>
class Stack
{
private:
    std::vector<Type> s;
    int N;
public:
    void resize(int capacity);
};

template <class Type>
void Stack<Type>::resize(int capacity)
{
    s.resize(capacity);
    if (N >= s.size())
        N = s.size()-1;
}