10

私は C++ で "ビッグ スリー" を学ぼうとしています.. "ビッグ スリー" の非常に単純なプログラムを作成することができました.. しかし、オブジェクト ポインターの使用方法がわかりません.. 以下は私の最初の試みです.

これを書いていて疑問が…

質問

  1. これは、デフォルトのコンストラクターを実装する正しい方法ですか? 持つ必要があるかどうかはわかりません。しかし、ポインターを使用したコピーコンストラクターに関する別のスレッドで見つけたのは、コピーコンストラクターでアドレスをコピーする前に、そのポインターにスペースを割り当てる必要があるということです..
  2. コピーコンストラクターでポインター変数を割り当てる方法は? Copy Constructor の書き方が間違っているかもしれません。
  3. コピー コンストラクターと operator= の両方に同じコード ( return を除く) を実装する必要がありますか?

  4. デストラクタでポインタを削除する必要があると言うのは正しいですか?

    class TreeNode
{
public:  
   TreeNode(); 
   TreeNode(const TreeNode& node);
   TreeNode& operator= (const TreeNode& node);
   ~TreeNode();
private:
   string data;
   TreeNode* left;
   TreeNode* right;
   friend class MyAnotherClass;
};

実装

TreeNode::TreeNode(){

    data = "";  

}

TreeNode::TreeNode(const TreeNode& node){
     data = node.data;

     left = new TreeNode();
     right = new TreeNode();

     left = node.left; 
     right = node.right;
}

TreeNode& TreeNode::operator= (const TreeNode& node){
     data = node.data;
     left = node.left;
     right = node.right;
     return *this;
}

TreeNode::~TreeNode(){
     delete left;
     delete right;
}

前もって感謝します。

4

5 に答える 5

22

デストラクタのポインタを削除する必要があると言っているのは正しいですか？

このようなオブジェクトを設計するときはいつでも、最初に質問に答える必要があります。オブジェクトは、そのポインタが指すメモリを所有していますか？はいの場合、明らかにオブジェクトのデストラクタはそのメモリをクリーンアップする必要があるため、はい、deleteを呼び出す必要があります。そして、これは与えられたコードに対するあなたの意図のようです。

ただし、場合によっては、存続期間が他の何かによって管理されることになっている他のオブジェクトを参照するポインターが必要になることがあります。その場合、プログラムの他の部分の義務であるため、deleteを呼び出したくありません。さらに、これにより、コピーコンストラクターと代入演算子に入る後続のすべてのデザインが変更されます。

各TreeNodeオブジェクトに左右のオブジェクトの所有権を持たせたいという前提で、残りの質問に答えていきます。

これはデフォルトのコンストラクターを実装する正しい方法ですか？

いいえ。leftとrightポインタをNULL（または必要に応じて0）に初期化する必要があります。初期化されていないポインタは任意の値を持つ可能性があるため、これが必要です。コードがデフォルトでTreeNodeを構築し、それらのポインターに何も割り当てずにそれを破棄した場合、その初期値が何であれ、deleteが呼び出されます。したがって、この設計では、これらのポインターが何も指していない場合は、それらがNULLに設定されていることを保証する必要があります。

コピーコンストラクターでポインター変数を割り当てる方法は？コピーコンストラクタで書いた方法が間違っている可能性があります。

この行left = new TreeNode();は、新しいTreeNodeオブジェクトを作成し、leftそれを指すように設定します。この行left = node.left;は、TreeNodeオブジェクトがnode.left指すものを指すようにそのポインターを再割り当てします。それには2つの問題があります。

問題1：その新しいTreeNodeを指すものはありません。何も破壊できないため、失われ、メモリリークになります。

問題2：両方ともleft、node.left同じTreeNodeを指すことになります。つまり、コピーで構築されているオブジェクトと値を取得しているオブジェクトは、どちらも同じTreeNodeを所有していると見なし、デストラクタでそのオブジェクトに対してdeleteを呼び出します。同じオブジェクトに対してdeleteを2回呼び出すことは常にバグであり、問​​題（クラッシュやメモリの破損など）を引き起こします。

各TreeNodeはその左ノードと右ノードを所有しているため、おそらく最も合理的な方法はコピーを作成することです。したがって、次のようなものを記述します。

TreeNode::TreeNode(const TreeNode& node)
    : left(NULL), right(NULL)
{
    data = node.data;

    if(node.left)
        left = new TreeNode(*node.left);
    if(node.right)
        right = new TreeNode(*node.right);
}

コピーコンストラクターとoperatior=の両方に同じコード（returnを除く）を実装する必要がありますか？

ほぼ確実に。または、少なくとも、それぞれのコードの最終結果は同じである必要があります。コピーの作成と割り当ての効果が異なると、非常に混乱します。

編集-上記の段落は次のようになります。データが他のオブジェクトからコピーされるという点で、それぞれのコードの最終結果は同じである必要があります。これには、非常によく似たコードが含まれることがよくあります。ただし、代入演算子は、おそらく何かがすでに代入されているかどうかを確認し、それらをクリーンアップする必要がleftありrightます。結果として、自己割り当てに注意する必要がある場合もあります。そうでない場合は、自己割り当て中に問題が発生しないように記述する必要があります。

実際、メンバー変数を操作する実際のコードが1つの場所にのみ書き込まれるように、一方を他方を使用して実装する方法があります。SOに関する他の質問では、これなどが議論されています。

于 2010-09-18T06:08:03.487 に答える
8

さらに良いと思います

 TreeNode::TreeNode():left(NULL), right(NULL)
 {
   // data is already set to "" if it is std::string
 }

さらに、代入操作でポインタ「左」と「右」を削除する必要があります。そうしないと、メモリリークが発生します。

于 2010-09-18T05:29:59.193 に答える
4

これは私が行う方法です:
同じオブジェクトで2つのリソースを管理しているため、これを正しく行うことはもう少し複雑になります (これが、オブジェクトで複数のリソースを管理しないことをお勧めする理由です)。コピー/スワップ イディオムを使用する場合、複雑さはコピー コンストラクターに限定されます (強力な例外保証を正しく取得するのは簡単ではありません)。

TreeNode::TreeNode()
    :left(NULL)
    ,right(NULL)
{}

/*
 * Use the copy and swap idium
 * Note: The parameter is by value to auto generate the copy.
 *       The copy uses the copy constructor above where the complex code is.
 *       Then the swap means that we release this tree correctly.
 */ 
TreeNode& TreeNode::operator= (const TreeNode node)
{
     std::swap(data,  node.data);
     std::swap(left,  node.left);
     std::swap(right, node.right);
     return *this;
}

TreeNode::~TreeNode()
{
     delete left;
     delete right;
}

今難しい部分：

/*
 * The copy constructor is a bit harder than normal.
 * This is because you want to provide the `Strong Exception Guarantee`
 * If something goes wrong you definitely don't want the object to be
 * in some indeterminate state.
 *
 * Simplified this a bit. Do the work that can generate an exception first.
 * Once all this has been completed we can do the work that will not throw.
 */   
TreeNode::TreeNode(const TreeNode& node)
{
    // Do throwable work.
    std::auto_ptr<TreeNode>  nL(node.left  == null ? null : new TreeNode(*node.left));
    std::auto_ptr<TreeNode>  nR(node.right == null ? null : new TreeNode(*node.right));

    // All work that can throw has been completed.
    // So now set the current node with the correct values.
    data  = node.data;
    left  = nL.release();
    right = nR.release();
}
于 2010-09-18T16:23:33.333 に答える
1

単純なポインターの代わりに、ライブラリのブーストからのブースト:: shared_ptr（使用できる場合）も提案できますか？無効なポインタ、深いコピーなどで発生する可能性のある多くの問題を解決します

于 2010-09-18T06:11:14.117 に答える
1

Is this the correct way to implement the default constructor?

No, calling delete on something that has not been allocated using new invokes Undefined Behavior (in most cases it results to crash of your application)

Set your pointers to NULL in your default constructor.

TreeNode::TreeNode(){
  data = "";   //not required since data being a std::string is default initialized.
  left = NULL;
  right = NULL;   
}

I don't see such problems with the rest of your code. Your assignment operator shallow copies the nodes whereas your copy constructor deep copies them.

Follow a suitable approach as per your requirement. :-)

EDIT:

Instead of assigning pointers in your default constructor use an initialization list

于 2010-09-18T05:14:40.963 に答える