3

私は問題があります。ポインタを含むオブジェクトクラスのクローンを作成する必要があります。問題の例は、次のコードにあります。

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

class CPoint
{
protected:
    int m_x;
    int m_y;

    int *m_p;

public:
    CPoint();
    CPoint(int x, int y);
    ~CPoint();

    CPoint*         clone();
    static CPoint*  clone(CPoint& p);

    int getX();
    int getY();
    void setX(int x);
    void setY(int y);

    void toString();
};

int CPoint::getX()
{
    return m_x;
}

int CPoint::getY()
{
    return m_y;
}

void CPoint::setX( int x )
{
    m_x = x;
}

void CPoint::setY( int y )
{
    m_y = y;
}

void CPoint::toString()
{
    std::cout << "(" << m_x << ", " << m_y<< ", " << *m_p << ")" << std::endl;
}

CPoint::CPoint( int x, int y )
{
    m_x = x;
    m_y = y;    

    m_p = new int();
    *m_p = x + y;
}

CPoint::CPoint()
{
    m_p = new int();
    *m_p = 1000;
}

CPoint* CPoint::clone()
{
    CPoint *p = new CPoint();
    *p = *this;
    return p;
}

CPoint* CPoint::clone( CPoint& p )
{
    CPoint *q = new CPoint();
    *q = p;
    return q;
}

CPoint::~CPoint()
{
    if (m_p) {
        delete m_p;
        m_p = NULL;
    }
}

int _tmain(int argc, _TCHAR* argv[])
{
    CPoint *p1 = new CPoint(10, 20);
    CPoint *p2 = new CPoint(30, 40);

    p1->toString();
    p2->toString();

    CPoint *p3;
    p3 = CPoint::clone(*p1);

    p3->toString();

    CPoint *p4;
    p4 = p2->clone();

    p4->toString();

    p1->setX(50);
    p1->setY(60);
    p2->setX(80);
    p2->setY(90);

    p3->toString();
    p4->toString();

    delete p1;
    delete p2;
    delete p3;
    delete p4;

    int a;
    std::cin >> a;

    return 0;
}

変数で私が抱えている問題m_pp1オブジェクトのクローンを作成する場合p2、メモリアドレスp3とは異なりますが、アドレスは同じです。明らかに、削除すると、削除は失敗します。とと同じです。p4p1p3m_pp1p3p2p4

CPointクラスオブジェクトのクローンを作成するにはどうすればよいですか?

4

6 に答える 6

6

他のJavaのような言語のルールをC++に適用しているようです。
これは根本的な問題であり、長期的にはあらゆる種類の問題につながるでしょう。

C++のイディオムを学ぶ必要があります。

C ++では、C-Stringインターフェースではなく、C ++文字列(std :: string)を使用します。

#include <string.h>   // C-Interface

// What you really want
#include <string>     // C++ Interface

クラスにポインタが含まれている場合は、おそらく何か問題があります。RAWポインターは、その寿命を正しく制御するために、スマートポインター(またはコンテナー)でラップする必要があります。ビジネスクラスにポインタを置くと、関心の分離の原則が破られます。

class CPoint
{
    protected:
        int m_x;
        int m_y;

        int *m_p;  // What is it supposed to be?
                   // Who owns it?

あなたのクラスはポインタを持っていたので、三つのルールを破りました。
このクラスでポインターを管理したい場合(そして(関心の分離を破る)しない場合)、3のルール(C ++ 11では5のルール)を実装する必要があります(調べてください)。RAWポインタの処理方法を知りたい場合は、https://stackoverflow.com/a/1846409/14065をご覧ください。

クローンメソッドは必要ありません。これがコピーコンストラクタの目的です。クローンを作成する必要のあるクラスを作成していません(そうでない場合は、仮想のデストラクタが含まれているはずです)。あなたのクラスは多形ではなく、から派生することはありません。したがって、コピーコンストラクタは完全に機能します。

CPoint*         clone();
static CPoint*  clone(CPoint& p);

// Copy constructor looks like this:
CPoint(CPoint const& rjs)

// Assignment operator looks like this:
CPoint& operator=(CPoint& rhs)

ただし、RAWポインタを適切なクラスで正しくラップする場合は、これは必要ありません。コンパイラが生成したこれらのメソッドのデフォルトバージョンは正常に機能します。

カプセル化を完全に破壊する良い方法。

int getX();
int getY();
void setX(int x);
void setY(int y);

文字列に!うんこ。本当に必要なのはシリアル化方法です。

void toString();

// serializer look like this:

friend std::ostream& operator<<(std::ostream& stream, CPoint const& data)
{
     // Convert CPoint (data) to the stream.
     return stream;
}

C ++では、必要がない限り、オブジェクトを動的に作成しません。
そして、ここであなたはする必要はありません。ローカルオブジェクトの作成は、例外が存在する場合でもその寿命が保証されるため、より適切に機能します。

// Rather than dynamically creating them
CPoint *p1 = new CPoint(10, 20);
CPoint *p2 = new CPoint(30, 40);

// Just declare two local variables:
CPoint  p1 = CPoint(10, 20);
CPoint  p2(30, 40);           // Alternative to the above but means the same.

// Much better to use operator<<
// Also shows the functions are badly named. You are not converting to string.
// but rather printing them to a stream.
p1->toString();
p2->toString();

std::cout << p1;
myFileStream << p2;  // allows you to easily specify the actual stream.

コピーコンストラクタは、オブジェクトのコピーに非常に適しています

CPoint *p3;
p3 = CPoint::clone(*p1);

// If we were still using pointers. 
CPoint* p3 = new CPoint(p1);

// But much nicer to not even use pointers
CPoint  p3(p1);

関数で削除する手動呼び出しを目にした場合、通常は設計ミスです。

delete p1;
delete p2;
delete p3;
delete p4;

クラスのようにスマートポインター(またはコンテナー)でそれらをラップするポインターがある場合、それらは例外的に安全に使用できます。これは、ローカルオブジェクトの場合、デストラクタが呼び出されることが保証されているため、オブジェクトがスコープ外になると、ポインタが正しく削除されるためです。現在、このコードは例外安全ではなく、例外が伝播して渡された場合にリークします。

小さな注意:main()は特別です。戻り値を指定しない場合、コンパイラーはあなたのreturn 0;ために植え付けます。アプリケーションにエラー状態がない場合は、この機能を他の開発者へのサインとして使用して、コードが常に正常に終了することを確認してください。

return 0;

私はこのように書き直します:

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

class CPoint
{
protected:
    int m_x;
    int m_y;

    std::vector<int> m_p;

public:
    // If you don't explicitly initialize m_x and m_y them
    // they will have indeterminate (random) values.
    CPoint()             : m_x(0), m_y(0) {m_p.push_back(1000);}
    CPoint(int x, int y) : m_x(x), m_y(y) {m_p.push_back(x + y);}

    int getX()        { return m_x;}
    int getY()        { return m_y;}
    void setX(int x)  { m_x = x;}
    void setY(int y)  { m_y = y;}

    friend std::ostream& operator<<(std::ostream& stream, CPoint const& d)
    {
        return stream << "(" << d.m_x << ", " << d.m_y<< ", " << d.m_p[0] << ")" << std::endl;
    }
};




int main(int argc, char* argv[])
{
    CPoint p1(10, 20);
    CPoint p2(30, 40);

    std::cout << p1 << p2;

    CPoint p3(p1);

    std::cout << p3;

    CPoint p4(p2);
    std::cout << p4;

    p1.setX(50);
    p1.setY(60);
    p2.setX(80);
    p2.setY(90);

    std::cout << p1 << p2 << p3 << p4;
    int a;
    std::cin >> a;
}
于 2012-10-17T14:17:20.523 に答える
1

直接のデータメンバーm_xとを浅くコピーすることに加えてm_y、ポインタメンバーをディープコピーする必要がありますm_p。このクラスのコンストラクターや実際に指しているものを示していないので、それがの配列の最初の要素を指しているm_pと仮定します。これをディープコピーするには、次のことが必要です。m_pint

  1. int元の配列と同じ(またはそれ以上の)サイズの新しい配列をインスタンス化します
  2. 各要素を元の配列から新しい配列にコピーします
  3. m_pこの新しい配列の最初の要素を指すように、複製されたオブジェクトに設定します

これがどのように行われるかの例:

CPoint* CPoint::clone(CPoint& rhs)
{
  CPoint* ret = new CPoint;
  ret->m_x = rhs.m_x;
  ret->m_y = rhs.m_y;

  size_t m_p_count = /* somehow determine the size of rhs.m_p */;
  ret->m_p = new int[m_p_count];
  std::copy(&rhs.m_p[0], &rhs.m_p[m_p_count], ret->m_p);

  return ret;
}

コードに関するいくつかの注意事項:

  1. vector<int>の配列への生のポインタの代わりに使用する方が良いでしょうint
  2. #1を除いて、生のポインターの代わりにスマートポインターを使用する必要があります
  3. 上記のコードには、配列のサイズを決定する方法がありません。--を使用した場合、これは簡単です。vector<int>を呼び出すだけvecctor<int>::size()です。明らかに、配列のコピーを作成するには、配列のサイズを知る必要があります。
  4. clone()関数は通常、基本クラスのポインターを介してポリモーフィックオブジェクトのコピーを作成する場合にのみ役立ちます。クラスとその使用法はこのカテゴリに分類されclone()ないため、関数はそもそも正しい方法ではありません。代わりに、コピーコンストラクタとコピー代入演算子の使用を検討してください。デストラクタも実装することを忘れないでください。さらに良いことに、これらすべてを完全に避け、ゼロのルールに従ってください。
于 2012-10-17T11:27:29.740 に答える
1

この例では整数ですが、任意のタイプにすることができます。私が持っていた質問は、別のタイプへのポインタを含むオブジェクトのクローンを作成することでした。

ここには基本的に2つの状況があると思います。含まれているオブジェクトがポイントされたオブジェクトを所有するようにしたい場合。または、含まれているオブジェクトがポイントされたオブジェクトを所有することを望まない場合。

非所有から始めましょう。所有していないポインタを表すためにC++が提供するツールは何ですか?まあ、通常のポインタは所有していません。そして、どのようにして通常のポインタをコピーしますか?あなたは何もしません。コンパイラーにそれを処理させ、自由に使用できる正しいコピーコンストラクターを生成します(そして、その間、コンパイラーにデストラクタも生成させます)。

そして、所有するのはどうですか?ポインタを所有するためのツールは何ですか?ほとんどの場合、そのためのポインタも必要ありません。値を直接格納し、コンパイラに正しいコピーコンストラクタ(およびデストラクタも!)を生成させます。提供されている例では、int m_p;うまく機能します。

多態的な基本クラスが関係している場合、この状況には厄介な問題があります。コピーするとスライスが発生する可能性があります。C ++はこの状況に対応するツールを提供しますか?悲しいことに、そうではありません。手で書かなければなりません。ただし、自分に有利に働き、これらの懸念をクラスの他のメンバーと混同しないでください(単一責任原則)。

単一のポインターを所有し、破棄時にクリーンアップし、コピーコンストラクターでポリモーフィックコピー(一般的なイディオムには仮想クローン関数が含まれます)を実行する再利用可能なクラス(ボーナスポイント:テンプレートにする)を記述します。次に、その再利用可能なクラスの値をあなたの中に入れてくださいCPoint...あなたはそれを推測しました!コンパイラーに正しいコピーコンストラクターを生成させます。

于 2012-10-17T13:52:06.137 に答える
0

オブジェクトの各インスタンスは、それが指すオブジェクトを「所有」していますか、それとも他の何かが所有する共通のオブジェクトを参照していますか?

所有権のケースがある場合、各インスタンスは個別のコピーを指している必要があります。つまり、ポインタをコピーする必要はありません。ポインタが指すオブジェクトのクローンを作成し、この新しいオブジェクトをコピーのポインタに割り当てる必要があります。

于 2012-10-17T11:19:59.607 に答える
0

内のすべてのポインタにメモリを再割り当てしCPoint、それらのデータを新しいメモリにコピーする必要があります。あなたの場合、あなたは以下の操作をしなければなりません:

CPoint clone()
{
   CPoint p;
   p = *this;
   p.m_p = new int();
   *p.m_p = *m_p;
   return p;
}
于 2012-10-17T11:20:06.897 に答える
-1

m_pが(配列全体ではなく)1つの整数のみを指していると仮定すると、クローン作成は次のように実行できます。

CPoint* CPoint::clone()
{
    CPoint* cloned = new CPoint(m_x, m_y);
    if (m_p)
    {
        cloned->m_p = new int;
        *cloned->m_p = *m_p;
    }
    return cloned;
}

このようなメンバーポインタは、NULL値を持つ可能性を追加することを唯一の目的としていることに注意してください。これは別の意味を持つ可能性があります。

また、メモリリークとヒープの破損を回避するために、次のことを行う必要があることにも注意してください。

  • コピーコンストラクタと代入演算子は「無効」にする必要があります(プライベートとして宣言)
  • デストラクタはm_pを削除する必要があります。
于 2012-10-17T11:21:00.877 に答える