2016-10-13 17 views
2

次のコードは正しくビルドされますが、実行するとプログラムがクラッシュします。誰かが私に何が間違っているか教えてもらえますか?私はDeleteNode関数に何か問題があると思われる。単一リンクリストが機能しない(C++)

#include <iostream> 
#include <cstdlib> 

using namespace std; 

class list { 
private: 
    typedef struct node { 
     int data; 
     node* next; 
    }* nodePtr; //this means that 'nodePtr' will mean a pointer to the struct node 

nodePtr head; 
nodePtr current; 
nodePtr temp; 

public: 
list() { //constuctor 
    head = NULL; 
    current = NULL; 
    temp = NULL; 
}; 

void AddNode(int addData) //to add a particular data value 
{ 
    nodePtr n= new node; 
    n->next = NULL; 
    n->data = addData; 

    if (head != NULL) { //if a list is already set up 
     current = head; 
     while (current->next != NULL) { //to get to the last node in the list 
      current = current->next; 
     } 
     current->next = n; 
    } 
    else { // if list is not created 
     head = n; //new node is front of the list 
    } 
} 

void DeleteNode(int delData) //to delete a particular data value 
{ 
    nodePtr delPtr = NULL; 
    temp = head; 
    current = head; 

    while (current != NULL && current->data!=delData) { //pass through whole list && find value 
     temp = current; 
     current = current->next; 
    } 

    if (current = NULL) { //data value not found in list 
     cout << delData << " was not in the list." << endl; 
     delete delPtr; //to free up memory space 
    } 
    else { 
     delPtr = current; 
     current = current->next; 
     temp->next = current; //to reconnect list 

     if (delPtr == head) { 
      head = head->next; 
      temp = head; 
     } 

     delete delPtr; 
     cout << "The value " << delData << "was deleted." << endl; 
    } 
} 

void PrintList() //to print all the data values 
{ 
    current = head; 

    while (current != NULL) { //to go through the data valued of the list 
     cout << current->data << endl; 
     current = current->next; 
    } 
} 

}; 



int main() 
{ 
    list Shahzad; 

    Shahzad.AddNode(2); 
    Shahzad.AddNode(78); 
    Shahzad.AddNode(28); 
    Shahzad.AddNode(2398); 

    Shahzad.DeleteNode(78); 
    Shahzad.PrintList(); 
    return 0; 
} 
+1

ようこそスタックオーバーフロー!デバッガを使用してコードをステップ実行する方法を学ぶ必要があるようです。良いデバッガを使用すると、プログラムを1行ずつ実行し、どこからずれているかを確認することができます。これはプログラミングをする場合に不可欠なツールです。詳しい読書:** [小さなプログラムをデバッグする方法](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/)** – NathanOliver

+1

このような問題を解決する適切なツールは、デバッガ。スタックオーバーフローを尋ねる前に、コードを一行ずつ進める必要があります。詳しいヘルプは、[小さなプログラムをデバッグする方法(Eric Lippert)](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/)を参照してください。最低限、問題を再現する[最小、完全、および検証可能](http://stackoverflow.com/help/mcve)の例と、その問題を再現するためのデバッガ。 –

+0

私はそれをすべて読んでいませんでしたが、うまくできない 'if(current = NULL)'も – RyanP

答えて

2

あなたの最初の問題は、次の行である:

if (current = NULL) 

あなたはからnullを割り当て、実際ですこの時点で210。

これは実際には次のようになります。ここにすべての提案から離れて

if (current == NULL) 
0

ノードが見つからない場合の削除機能では、delPtrを削除しています。

ただし、delPtrはインスタンス化されず、割り当てられていないので、存在しないものを削除しようとしていません。

この問題を回避するには、if文で必ずポインタの削除を囲んでください。 、これは理解するあなたのクラスが簡単になります.hクラスメンバーが宣言されているファイルおよびクラスメンバーが実装されている.cppにあなたのコードを分離することを検討し、可能性のあるエラー:

if (delPtr) delete delPtr; 
+0

それは確かに教えて問題を引き起こしませんか? 'if(delPtr)delete delPtr;'は冗長です。 'delete'は' nullptr'で透過的に動作します。 –

1

まず、いくつかのコードとファイル管理の発言はこれを試してみてください見つけやすくなります。

第2に、ポインタを含む構造体を扱う際の一般的なアドバイスは、適切なリソース管理に注意することです。つまり、ポインタの定義、初期化、および削除には注意が必要です。 「unique_ptrがスコープの外に出たとき、ポインタを通じてオブジェクトの唯一の所有権を保持し、そのオブジェクトを破壊する」第三

ますstd::unique_ptr:あなたが初心者であれば、のようなすでに提供スマートポインタ施設の利用を検討、のような些細なエラーを取り除くために使用デバッガ:によって追加の不正確さが含まれてい

if (current = NULL) 

ではなく、リテラルnullptrポインタのNULLの使用で表現し​​ました。

最後に、あなたが最初の実装を完了し、だけにして、さらにクラスの拡張を進めた後、別途メンバ関数のそれぞれをチェックし、そうでなければ、あなたの仕事を行います複数のソースからの誤差の累積を危険にさらすことは非常に困難


0

、あなたが早期にバグをキャッチするために、いくつかの安全なプログラミング手法を使用することができます。EXのために

:あなたは

をミスタイプした場合、ここで

if (NULL == current) 

:あなたはLHSと、このようなRHS上の変数にチェックされている値を書き込むしようとし、その代わり

if (current = NULL) 

を書きました

if (NULL = current) 

コンパイラが不平を言います。実行時のバグではなく、コンパイル時のバグがあります。これは、見つけてデバッグする方がはるかに簡単です。

関連する問題