Сделал первый проход исправлений в вашем коде:
template <class T>
class Stack
{
private:
int* values;
int capacity;
int itemsOnStack;
public:
//Stack() :
//{
// Stack(32); // doesn't do what you expect. This would create an unnamed temporary stack object
//}
Stack(const int sz = 32) // C++ doesn't yet have delegating constructors. You can't call one ctor from another. But in this case, a simple default parameter can be used instead
: values(new T[sz]), capacity(sz), itemsOnStack() {} // use the initializer list for initializing members
~Stack()
{
delete[] values; // you allocated it, so you delete it as well
}
////////////////////
void Push(const T& item)
{
values[itemsOnStack] = item; // cleaner syntactically than your version
// *(values + itemsOnStack) = item;
++itemsOnStack; // prefer pre-increment by default.
if(itemsOnStack > capacity) // you need to check this before writing the element. Move this to the top of the function
{
int newCapacity = capacity * 2;
// what's this supposed to do? You're just copying pointers around, not the contents of the array
T* temp = new T[newCapacity ];
std::copy(values, values+capacity, temp); // copy the contents from the old array to the new one
delete[] values; // delete the old array
values = temp; // store a pointer to the new array
capacity = newCapacity;
}
}
///////////////////
T Pop()
{
T result = Peek(); // you've already got a peek function. Why not use that?
--itemsOnStack;
return result;
}
///////////////////
T Peek()
{
if(itemsOnStack > 0)
{
int current = itemsOnStack - 1;
return values[current]; // again, array syntax is clearer than pointer arithmetics in this case.
}
// return NULL; // Only pointers can be null. There is no universal "nil" value in C++. Throw an exception would be my suggestion
throw StackEmptyException();
}
///////////////////
int Count()
{
return itemsOnStack;
}
///////////////////
int Capacity()
{
return capacity;
}
///////////////////
bool IsEmpty()
{
return itemsOnStack == 0;
}
};
Осталось исправить:
Копировать конструктор и оператор присваивания. На данный момент, если я попытаюсь сделать это, он ужасно сломается:
Stack<int> s;
Stack<int> t = s; // no copy constructor defined, so it'll just copy the pointer. Then both stacks will share the same internal array, and both will try to delete it when they're destroyed.
Stack<int> u;
u = s; // no assignment operator, so much like above, it'll blow up
Const правильность:
Разве я не могу позвонить Peek()
или Count()
на const Stack<T>
?
Срок службы объекта:
Удаление элемента из стека не вызывает деструктор элемента. Выдвижение элемента не вызывает конструктор элемента. Простое расширение массива вызывает конструктор по умолчанию сразу для всех новых элементов. Конструктор должен вызываться, когда пользователь вставляет элемент, а не раньше, и деструктор сразу после удаления элемента.
И, ну, правильное тестирование:
Я не компилировал, не запускал, не проверял и не отлаживал это каким-либо образом. Поэтому я, скорее всего, пропустил некоторые ошибки и представил несколько новых. ;)