2011-03-16 10 views
1

先週、クラスとベクトルを使用して文字列のセットを管理するプログラムを作成しました。私はこれを100%完成させることができました。今週は、クラス内の文字列を格納するために使用したベクトルを、単純な単一リンクリストに置き換える必要があります。単一リンクリストの非常に基本的な実装のためのコピーコンストラクタに関する助けが必要

この関数は、基本的にユーザーが空の文字列のセットを宣言することを可能にし、1つの要素のみで設定します。メインファイルには、要素がsetNameとstrSet(クラス)を含む構造体であるベクトルがあります。

私の問題です:クラスのコピーコンストラクタを扱います。コピーコンストラクタを削除/コメントアウトするとき、空のセットまたは単一のセットを必要なだけ宣言し、問題なく値を出力できます。しかし、私は明らかに私がプログラムの残りの部分を実装するときのコピーコンストラクタが必要になることは知っています。コピーコンストラクタを残しておくと、1つまたは複数のセットを宣言してその値を出力できます。しかし、2番目のセットを宣言し、最初の2つのセットのどちらかを出力しようとすると、セグメンテーションフォルトが発生します。さらに、2セット以上宣言しようとすると、セグメンテーションフォルトが発生します。どんな助けもありがとう!ここで

は、すべての非常に基本的な実装のための私のコードです:ここでは

はsetcalc.cppです:(メインファイル)ここで

#include <iostream> 
#include <cctype> 
#include <cstring> 
#include <string> 
#include "strset2.h" 

using namespace std; 

// Declares of structure to hold all the sets defined 
struct setsOfStr { 
    string nameOfSet; 
    strSet stringSet; 
}; 

// Checks if the set name inputted is unique 
bool isSetNameUnique(vector<setsOfStr> strSetArr, string setName) { 
    for(unsigned int i = 0; i < strSetArr.size(); i++) { 
     if(strSetArr[i].nameOfSet == setName) { 
      return false; 
     } 
    } 
    return true; 
} 

int main() { 
    char commandChoice; 
    // Declares a vector with our declared structure as the type 
    vector<setsOfStr> strSetVec; 

    string setName; 
    string singleEle; 
    // Sets a loop that will constantly ask for a command until 'q' is typed 
    while (1) { 
      cin >> commandChoice; 
     // declaring a set to be empty 
     if(commandChoice == 'd') { 
      cin >> setName; 
      // Check that the set name inputted is unique 
      if (isSetNameUnique(strSetVec, setName) == true) { 
       strSet emptyStrSet; 
       setsOfStr set1; 
       set1.nameOfSet = setName; 
       set1.stringSet = emptyStrSet; 
       strSetVec.push_back(set1); 
      } 
      else { 
       cerr << "ERROR: Re-declaration of set '" << setName << "'\n"; 
      } 
     } 
     // declaring a set to be a singleton 
     else if(commandChoice == 's') { 
      cin >> setName; 
      cin >> singleEle; 
      // Check that the set name inputted is unique 
      if (isSetNameUnique(strSetVec, setName) == true) { 
       strSet singleStrSet(singleEle); 
       setsOfStr set2; 
       set2.nameOfSet = setName; 
       set2.stringSet = singleStrSet; 
       strSetVec.push_back(set2); 
      } 
      else { 
       cerr << "ERROR: Re-declaration of set '" << setName << "'\n"; 
      } 
     } 
     // using the output function 
     else if(commandChoice == 'o') { 
      cin >> setName; 
      if(isSetNameUnique(strSetVec, setName) == false) { 
       // loop through until the set name is matched and call output on its strSet 
       for(unsigned int k = 0; k < strSetVec.size(); k++) { 
        if(strSetVec[k].nameOfSet == setName) { 
          (strSetVec[k].stringSet).output(); 
        } 
       } 
      } 
      else { 
       cerr << "ERROR: No such set '" << setName << "'\n"; 
      } 
     } 
     // quitting 
     else if(commandChoice == 'q') { 
      break; 
     } 
     else { 
      cerr << "ERROR: Ignoring bad command: '" << commandChoice << "'\n"; 
     } 
    } 
    return 0; 
} 

strSet2.hさ:

#ifndef _STRSET_ 
#define _STRSET_ 

#include <iostream> 
#include <vector> 
#include <string> 

struct node { 
    std::string s1; 
    node * next; 
}; 

class strSet { 

private: 
    node * first; 
public: 
    strSet(); // Create empty set 
    strSet (std::string s); // Create singleton set 
    strSet (const strSet &copy); // Copy constructor 
    // will implement destructor and overloaded assignment operator later 

    void output() const; 


}; // End of strSet class 

#endif // _STRSET_ 

ここにstrSet2.cpp(claの実装SS)

#include <iostream> 
#include <vector> 
#include <string> 
#include "strset2.h" 

using namespace std; 

strSet::strSet() { 
    first = NULL; 
} 

strSet::strSet(string s) { 
    node *temp; 
    temp = new node; 
    temp->s1 = s; 
    temp->next = NULL; 
    first = temp; 
} 

strSet::strSet(const strSet& copy) { 
    cout << "copy-cst\n"; 
    node *n = copy.first; 
    node *prev = NULL; 
    while (n) { 
     node *newNode = new node; 
     newNode->s1 = n->s1; 
     newNode->next = NULL; 
     if (prev) { 
      prev->next = newNode; 
     } 
     else { 
      first = newNode; 
     } 
     prev = newNode; 
     n = n->next; 
    } 
} 

void strSet::output() const { 
    if(first == NULL) { 
     cout << "Empty set\n"; 
    } 
    else { 
     node *temp; 
     temp = first; 
     while(1) { 
      cout << temp->s1 << endl; 
      if(temp->next == NULL) break; 
      temp = temp->next; 
     } 
    } 
} 

答えて

0

これは少し独特なります

strSet::strSet(string s) { 
    node *temp; 
    temp = new node; 
    temp->s1 = s; 
    temp->next = NULL; 
    first = temp; 
} 

を指しているものを '最初の' の場合すでに何か?あなたは効果的に前のリストを殺して、memリークを引き起こしています。

+0

関数がコンストラクタであるため、 'first'はこの関数が呼び出される前に既に何かを指していません。 – aschepler

+0

はい、そのコンストラクタです。 – Tesla

0

その引数が空のときにあなたのstrSetコピーコンストラクタは、メンバーfirstを割り当てません。これにより、未定義の動作が発生します。

また、編集前に表示されたstrSet割り当て演算子(operator=)は間違っていました。コピーコンストラクタを定義するのは良い考えではありませんが、デストラクタと代入演算子はコンパイラによって暗黙的に定義されます。 Rule of Threeを参照してください。

彼らは(この場合のように)かなりの管理を行う必要があるときビッグスリーを実装するための一般的な方法の1つのようなものになります。タイプは標準コンテナで使用

class strSet { 
private: 
    void cleanup(); 
    void create_from(const node* n); 
// ... 
}; 

strSet::~strSet() { cleanup(); } 

strSet::strSet(const strSet& copy) : first(NULL) { create_from(copy.first); } 

strSet& strSet::operator=(const strSet& rtSide) { 
    if (this != &rtSide) { 
     cleanup(); // trash old contents of *this 
     create_from(rtSide.first); // clone contents of rtSide 
    } 
    return *this; 
} 
+0

宣言すると、空のセットと言うことができます(つまり、入力コマンドが 'd'だった場合)、オーバーロードされた '='演算子は正しく使用されません。 3つの空集合を連続して宣言すると、なぜセグメンテーションフォルトが得られるのかが分かりません。私が宣言すれば、それはうまく動作し、その価値を出力することができます。私が2つのセットを宣言すると、値を出力しようとするとすぐに「セグメンテーションフォルト」が発生し、3セットを宣言しようとするとすぐに「セグメンテーションフォルト」が発生します。これは、オーバーロードされた '='演算子にアクセスする必要はありません。 – Tesla

+0

@イエス:あなたの他の問題を見つけて、私の答えを編集しました。しかし、 'operator ='を自分で宣言しなければ、コンパイラはあなたのためにコードを作ってくれます。 – aschepler

+0

さて、完璧!助けてくれてありがとう! – Tesla

1

をC++標準状態(のようなstd :: vector)はコピー可能で、かつ割り当て可能でなければなりません。

strSetクラスでカスタム代入演算子を実装していないので、コンパイラは単純なメンバーワイズコピーを生成します。あなたの場合、これは '最初の'ポインタが直接コピーされることを意味します。明らかに、これは2つのオブジェクトがセット内のノードを所有していることを意味し、2回解放されるとクラッシュすることになります。

いくつかのヒント:

  1. あなたのコピーコンストラクタ参照することにより、可能であればconst参照によってオブジェクトを渡すことで、最大読む

  2. と同じことを行うカスタム代入演算子を実装します。価値のあるものを渡すときに、コンテナや文字列を不必要に多くコピーしています。

ブールisSetNameUnique(constのベクトル& strSetArr、constの文字列&のsetName)

幸運:)

関連する問題