栈实现

Question

我想就堆栈的实现发表意见：

template <class T>
class Stack {
 public:
        void pop(T& element);
        void push(const T& element); 
 Stack();

 private:
         int elementCount_;
         int stackTop_;
  static const int  MAX_ELEMS = 10;
  T elementArray_[MAX_ELEMS]; 
};

template <class T>
Stack<T>::Stack() :
elementCount_(0),stackTop_(-1)
{
}

template <class T>
void Stack<T>::push(const T& element)
{
 if (stackTop_ == MAX_ELEMS-1){
            std::cout << "Stack is full" << "\n" ;
            return;    
 }
        elementArray_[++stackTop_] = element;
 elementCount_++;
}

template <class T>
void Stack<T>::pop(T& element)
{
    if (stackTop_ == -1){
     std::cout << "Stack is empty" << "\n";
     return;
    }
    elementCount_--;
    element =  elementArray_[stackTop_--];
}

Answer 1

以下是一些可以帮助您改进代码的东西。

抛出错误而不是打印错误消息

您的代码的用户可能正在创建一个没有命令行的GUI，并且不会希望您的代码打印而不是指示调用代码的错误。C++程序通常发出错误信号的两种方式是抛出异常(如果情况确实异常)或返回指示错误的值。

使用一致格式

张贴的代码有不一致的缩进，这使得它很难阅读和理解。选择一种风格并始终如一地应用它。

考虑添加功能

由于这是一个固定大小的堆栈，而不是动态大小的堆栈，所以允许用户指定该大小(可能是构造函数的默认参数)并对其进行查询是有意义的。另外，考虑添加诸如isFull()和isEmpty()之类的函数。

Answer 2

• 目前，如果堆栈是空的，pop不会更改element，也不会让代码知道它失败了。因此，使用堆栈的代码认为它弹出了操作之前的任何element。如果堆栈已满，push也是如此；代码无法知道它实际上并不只是推送一个元素。如果操作失败，则应抛出异常。(或者，您可以返回false。但是C++有异常，错误代码很容易被忽略。)
• 我不喜欢无缘无故地引用别人的话回来。“好的理由”包括已经返回了一个值(而你没有)。
• stackTop_和elementCount_ (至少目前)是多余的。如果不以相同的方式更改另一个，那么elementCount_总是等于stackTop_ + 1。我建议消除elementCount_并从stackTop_中计算它，反之亦然。或者，更好的是，更改内容，使stackTop_成为元素计数--也就是说，它始终是要推送的下一个元素的索引。实际上，这会简化一些事情，让前提条件感觉不那么不稳定。模板<类T> void Stack::push(const & element) { //注意我们如何不再检查MAX_ELEMS-1。//由于堆栈顶部是元素计数，所以现在只是//“如果堆栈已经有MAX_ELEMS元素”。如果(stackTop_ == MAX_ELEMS) {抛出std：：runtime_error(“堆栈是满的”)；} elementArray_stackTop_++ =元素；}模板 voidStack：：pop(T& element) { //在这里相同。将“如果下一个索引为-1”转换为“如果有0元素”。如果(stackTop_ == 0) {抛出std：：runtime_error(“堆栈为空”)；}元素= elementArray_-stackTop_；}
• 据说，元素一旦弹出就不会被处理掉。对于非平凡类型的内置types...but来说，这并不是什么大问题，它可能会导致一些东西比它们应该保存的时间更长。(例如，一个Stack<std::string>会保留每个被推送的字符串的副本，直到弹出它并推动其他的东西来覆盖它，或者直到堆栈被销毁--以第一位为准)。