더 나은 프로그래밍 실무를 묻는 질문이 입력되었습니다

Question

저는 C++을 배우면서 실제 데이터 형식을 구현하기 시작했습니다. 첫 번째 스택 (이것은 마음에 봄 처음이었습니다). 몇 가지 프로그래밍을했는데 작동하지만 이제는 내가해야 할 일에 대한 의견이 필요합니다. 특정 물건이나 다른 프로 팁을 삭제하는 것과 같습니다. 나는 무엇을해야하고 왜 그렇게해야합니까?

template <class T> 
class Stack 
{ 
private: 
    int* values; 
    int capacity; 
    int itemsOnStack; 

public: 

    /////////////////// 
    Stack() 
    { 
     Stack(32); 
    } 

    /////////////////// 
    Stack(const int sz) 
    { 
     values = new T[sz]; 
     capacity = sz; 
     itemsOnStack = 0; 
    } 

    ~Stack() 
    { 
     values = 0; 
       // delete? 
    } 

    //////////////////// 
    void Push(const T& item) 
    { 
     *(values + itemsOnStack) = item; 

     itemsOnStack++; 

     if(itemsOnStack > capacity) 
     { 
      capacity *= 2; 

      T* temp = new T[capacity]; 
      temp = values; 
      values = new T[capacity]; 
      values = temp;   
     } 
    } 

    /////////////////// 
    T Pop() 
    { 
     if(itemsOnStack > 0) 
     { 
      int current = --itemsOnStack; 
      return *(values + current); 
     } 
     return NULL; // ? good? 
    } 

    /////////////////// 
    T Peek() 
    { 
     if(itemsOnStack > 0) 
     { 
      int current = itemsOnStack - 1; 
      return *(values + current); 
     } 
     return NULL; // find something better here or shouldnt? 
    } 

    /////////////////// 
    int Count() 
    { 
     return itemsOnStack; 
    } 

    /////////////////// 
    int Capacity() 
    { 
     return capacity; 
    } 

    /////////////////// 
    bool IsEmpty() 
    { 
     return itemsOnStack == 0; 
    } 
}; 

Answer 1

이 코드에 대한 수정의 첫 번째 패스를 했습니까 그것을 쓰기 전에 배열의 크기를 확인해야합니다 :

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 Stack<T>에 Peek() 또는 Count()를 호출 할 수 있어야하지 않나요를?

개체 수명 :
스택에서 요소를 팝핑해도 요소의 소멸자가 호출되지 않습니다. 요소를 푸시해도 요소의 생성자는 호출되지 않습니다. 단순히 배열을 확장하면 모든 새 요소에 대해 즉시 기본 생성자가 호출됩니다. 생성자는 사용자가 befre가 아닌 요소를 삽입 할 때 호출되어야하고, 요소가 제거되면 즉시 소멸자가 호출되어야합니다.

적절한 테스트 :
어떤 방식 으로든 컴파일, 실행, 테스트 또는 디버깅하지 않았습니다. 그래서 나는 몇몇 버그를 놓쳤을 가능성이 매우 높았고 몇 가지 새로운 버그를 소개했습니다. ;)

Answer 2

소멸자에서는 0으로 설정하는 대신 values을 삭제해야합니다. push 메서드에서

의 배열은 크기를 조정할 수 없습니다. 크기 조정 배열에

대한 추가 정보 : 요한이 언급 한 것을 http://www.daniweb.com/forums/thread13709.html#

Answer 3

이와 함께 몇 가지 이름을하려면 : 해당되는 경우

1.) 초기화 목록
2를 사용)는 const 멤버 함수를 사용합니다.
3.) 표준과보다 가깝게 정렬 된 함수 이름을 사용하십시오. (빈() IsEmpty() 대신)
4.) 당신은 거대한 메모리 누수가 있습니다.

Answer 4

Push() 당신의 소멸자에서 메모리를 파괴 : 당신은

Answer 5

특히 스택 객체를 복사, 할당 또는 푸시 할 때 많은 메모리 누수가 발생합니다. 학습 연습이라 할지라도 자신의 메모리 관리 (C++로 배워야하지만 한 번에 한 단계 씩)가 필요하다는 점을 명확하게 제시 할 것을 제안합니다. 대신 원시 C 스타일의 값 배열보다는 std :: vector 또는 std :: list를 사용하십시오. 이것은 소멸자, 복사 생성자, 할당 연산자 및 Push() 메서드가 메모리를 누설하지 않는다는 것을 의미합니다. 기본적으로 올바른 작업을 수행합니다 (나는 최소 서프라이즈의 원칙). C++ 학습에서 가장 중요한 부분 중 하나는 STL을 사용하는 방법을 아는 것입니다. 처음에는 포인터와 새로운 []에 대해 걱정하고 나중에 []를 삭제하십시오.

복사 생성자, 할당 연산자 및 소멸자를 읽으면서 값이 실제로 복사되고 삭제되는 방법을 알 수 있습니다. 클래스 작성하십시오 :

class Noisy { 
    public: 
    Noisy() { std::cout << "Constructor" << std::endl; } 
    ~Noisy() { std::cout << "Destructor" << std::endl; } 
    Noisy& operator=(const Noisy& other) {std::cout << "Assign" << std::endl; } 
    Noisy(const Noisy& other) {std::cout << "Copy constructor" << std::endl; } 
}; 

을 다음 Stack<Noisy>에 몇 가지 작업을 수행합니다

Stack<Noisy> stack(10); 
stack.Push(Noisy()); 
Stack<Noisy> otherStack(stack); 
Stack<Noisy> thirdStack; 
thirdStack=stack; 

을하고 있는지 당신이 콘솔에 표시되는 내용과 일치를 수행해야한다고 생각 무엇을 기대 창문.

Answer 6

멤버 변수 네 개의 명명 규칙을 사용해야합니다. 우리는 등

을

int *m_values; 
int m_capactity; 

사용 그리고 왜

*(values + itemsOnStack) = item; 

을 나는 그들이 정확히 같은 일을 할 당신이

values[itemsOnStack] = item; 

을 의미 가정하지만 첫 번째가 훨씬 더 않는다 자연어