2017-11-16 16 views
-1

C++でリンクリストのpop関数を実装しようとしています。私のノードとリンクリストのクラスは次のようになります:C++ linkedList headは、リストが空でなくてもNULLを返します。

//Node class 
template <class T> 
class Node{ 
    public: 
     T data; 
     Node<T> *next; 

     Node(T data, Node<T> *next); 
     ~Node(); 
}; 

template <class T> 
Node<T>::Node(T data, Node<T> *next){ 
    this->next = next; 
    this->data = data; 
} 

template <class T> 
Node<T>::~Node(){ 
    delete this; 
} 

//LinkedList class 
template <class T> 
class LinkedList{ 
    public: 
     //fields 
     Node<T> *head; 
     int size; 

     //methods 
     LinkedList(); 
     void push(T data); 
     T pop(); 
     void printList(); 

}; 

template <class T> 
LinkedList<T>::LinkedList(){ 
    this->head = NULL; 
    this->size = 0; 
} 

template <class T> 
void LinkedList<T>::printList(){ 
    int i = 1; 
    while(head){ 
     std::cout<<i<<": "<<head->data<<std::endl; 
     head = head->next; 
     i++; 
    } 
} 

int main(){ 
    LinkedList<int> list; 

    for (int i = 1; i < 6; ++i) 
     list.push(i); 

    list.printList(); 

    for (int j = 0; j < 3; ++j){ 
     int output=list.pop(); 
     printf("popped: %d\n", output); 
    } 

    list.printList(); 
    return 0; 
} 

以下は私のポップ機能です。問題はthis-> headがNULLを返すことです。私はその価値を変えることも、データフィールドにアクセスすることもできません。私はprint文を使ってthis-> headがNULLを返すことを知りました。この問題を解決するにはどうすればよいですか?

以下は私のプッシュ機能です。私はこの関数をテストして正常に動作しますが、ノードポインタを適切に処理していない場合はお知らせください。

template <class T> 
void LinkedList<T>::push(T data){ 
    Node<T> *n = new Node<T>(data, this->head); 
    this->head = n; 
    this->size++; 
} 
+0

* hで〜ノード(); *あなたのためにコンパイル?とにかく、デストラクタを明示的にそのように呼び出すのはなぜですか?目的は何ですか? – hyde

+0

私が含まなかった〜ノード()のための関数を定義しました。私はノード・ポインタ* hを解放することによってメモリー・リークを止めようとしていましたが、今では自分のコードを見て、私はそれがまったく必要かどうかわかりません。それを指摘してくれてありがとう! – robt

+0

*私はメモリリークを止めようとしていました。*あなたは明示的にデストラクタを呼び出すのではなく、クラス内のリソースを適切に処理してメモリリークを止めます。 – PaulMcKenzie

答えて

2
template <class T> 
Node<T>::~Node(){ 
    delete this; 
} 

これには、非常に非常に間違っています。あなたは完全にそれを取り除く必要があります。

さらに重要なことは、printList()は、そうでなければならない場合はheadです。そのため、pop()が呼び出されたときにheadがNULLになっています。リストを反復するためにローカルNode*変数を使用します。

template <class T> 
void LinkedList<T>::printList(){ 
    int i = 1; 
    Node<T> *n = head; // <-- here 
    while(n){ 
     std::cout << i << ": " << n->data << std::endl; 
     n = n->next; 
     i++; 
    } 
} 

(すべての場合)また、pop()が正しくノードを解放していない、と常にsizeをデクリメントありません。

template <class T> 
T LinkedList<T>::pop(){ 
    Node<T> *h = this->head; 

    //if list is empty 
    if (this->size==0){ 
     return T(); // <-- not 0! or throw an exception instead... 
    } 

    //if list has only one node 
    if (this->size==1){ 
     T ret = h->data; 
     this->head = NULL; 
     this->size--; 
     delete h; // <-- add this! 
     return ret; 
    } 

    //if list has multiple nodes 

    T ret = this->head->data; 
    this->head = h->next; 
    this->size--; // <-- add this! 

    delete h; // <-- not h.~Node<T>()! 

    return ret; // <-- moved down here! 
} 

それとも単純に、この(別途size==1ケースを処理する必要はありません):それはより多くの代わりに次のようになります。

template <class T> 
T LinkedList<T>::pop(){ 
    Node<T> *h = this->head; 

    //if list is empty 
    if (!h){ 
     return T(); 
    } 

    //if list has any nodes 

    T ret = h->data; 
    this->head = h->next; 
    this->size--; 
    delete h; 

    return ret; 
} 
関連する問題