0

私はコードを持っています:

class Vector3D : public Vector{
protected:
    Point3D * start;
    Point3D * end;

public:
    ~Vector3D(){delete start; delete end;}
    Vector3D(Point3D * start, Point3D * endOrLength, bool fromLength){
        this->start = start;
        if(fromLength){
            this->end = new Vector3D(*start+*endOrLength); //the operator '+' is defined but I won't put it here,
            //because it's not important now
        }else
            this->end = endOrLength;
    }

    Point3D * getStart(){return start;}
    Point3D * getEnd(){return end;}
};

今、私はコードを持っています:

Vector3D v(new Point3D(1,2,3), new Point3D(2,3,4), 0); //FIRST CREATION
Vector3D v(new Point3D(1,2,3), new Point3D(1,1,1), 1); //SECOND CREATION

1 回目と 2 回目の作成で同じ Vector3D が得られますが、メモリ リークが発生する可能性があると思います。

それは本当ですか?そして、それを解決する方法は?そのようにするのはエレガントではないと思います:

...
if(fromLength){
    this->end = new Vector3D(*start+*endOrLength);
    delete endOrLength;
}else
...

const Point3D &endOrLenght を入れたほうがいいかもしれません。getStart/getEnd と同じ -ポインタを返す必要があります:

Point3D * getStart(){return start;}

または単に変数:

Point3D getStart()(return *start)

?

4

4 に答える 4

3

まず第一に、私は 3D ポイントのような値オブジェクトを動的に割り当てません。値を使用するだけで、多くの問題を解決できます。第 2 に、ベクトルを構築する方法が 2 つある場合は、2 つの異なるコンストラクターを提供するだけです。

class Vector3D {
public:
    Vector3D( const Point3D& s, const Point3D& e )
      : start( s )
      , end( e )
    {
    }

    Vector3D( const Point3D& s, const Vector3D& v )
      : start( s )
      , end( s + v )
    {
    }
}
private:
    Point3D start;
    Point3D end;
};

関数パラメーターに応じて 2 つの異なることを行う関数を持つことは、呼び出し側から理解することさえ困難です。この最後の 1 または 0 が何の役に立つのかを思い出すのは簡単ではありません。

よろしく、 トルステン

于 2012-07-02T11:52:04.837 に答える
2

ここでのコードは、これを処理する最良の方法ではないかもしれませんが、問題を直接修正するには:

Vector3D(Point3D * start, Point3D * endOrLength, bool fromLength){
    this->start = start;
    if(fromLength){
        this->end = new Vector3D(*start+*endOrLength); // I think you mean to use endOrLength here and not length.
        if (endOrLength)
            delete endOrLength;
    }else
        this->end = endOrLength;
}

問題のより良い解決策は、スマート ポインターを使用することだと思います。最善の解決策は、ポインターを置き換えることができるかどうかを確認することです。

class Vector3D : public Vector
{
protected:
    Point3D _start;
    Point3D _end;

public:
    Vector3D(const Point3D& start, const Point3D& endOrLength, bool fromLength) :
    _start(start),
    _end(fromLength ? Vector3D(start + endOrLength) : endOrLength)
    {
    }

    const Point3D& getStart() const { return _start; }
    const Point3D& getEnd() const { return _end; }
};
于 2012-07-02T11:47:28.063 に答える
1

設計は、その性質上 (すべての要素が孤立したままになる)、操作の局所性に違反する傾向があるため、効率が悪いと思われるかもしれません。一緒にあるものの間でのコピーは、疎なものの逆参照での操作よりもはるかに効率的です。

そして、あなたのベクトルはちょうど.... 2 * 3の数字なので、動的メモリのすべての複雑さを避け、通常の値のセマンティクスを使用してください。

より大きなサイズのオブジェクト (最大 16 の係数を持つ可能性のある 3D 射影行列など) に到達した場合は、動的メモリをクラスの内部にのみ処理するという考えを検討してください。あなたの場合、これは次のようになります

class Vector3d
{
    struct data
    { Point3d start, end; };

    Vector3d() :dp(new data) {}
    Vector3d(const Point3d& a, const Point3d& b) :p(new data({a,b})) {}

    Vector3d(const Vector3d& a) :p(new data(*a.p)) {}
    Vector3d(Vector3d&& a) :p(a.p) { a.p=nullptr; }
    Vector3d& operator=(Vector3d a) { delete p; p=a.p; a.p=nullptr; return *this; }
    ~Vector3d() { delete p; }

    const Poin3d& start() const { return p->start; }
    const Poin3d& end() const { return p->end; }

    //here define the compound assignment arithmetic
    //e.g.:
    Vector3d& operator+=(const Point3d& ofst)
    { p->start += ofst; p->end += ofst; return *this; }

    //alternativelly you can define assign in term of arithmetic
    Vector3d& operator-=(const Poinr3d& ofst)
    { *this = *this - ofst; return *this; } //will create a temporary that will be moved

private:
    data* p;
};

//and here define the point3d, vectopr3d and mixed arithmetic
//e.g.:
Vector3d operator+(Vector3d a, const Point3d& s)
{ a += s; return std::move(a); }

//this follow the alternative example
Vector3d operator-(const Vecotr3d& a, const Point3d& s)
{ return Vector3d(a.start()-s, a.end()-s); }

このようにして、動的メモリのすべての管理と、コピー (必要な場合) または移動 (可能な場合) の生成がクラスに残されます。他のすべては、標準の値のセマンティクスで機能します。

注: Poin3d には += -= + と - が定義されていると仮定しました ....

于 2012-07-02T12:15:26.007 に答える
1

正しい方法は次のとおりだと思います。

Vector3D(const Point3D& start, const Point3D& endOrLength, bool fromLength)

そうすれば、所有権にあいまいさはありません。また、メンバーはおそらくポインターであってはなりません。そうしないと、ダングリング ポインターが発生する可能性がありますが、データ メンバーになる可能性があります。

またはさらに良いことに、スマートポインター。

スマート ポインターを使用する場合は、get 関数からスマート ポインターを返すことができます。

于 2012-07-02T11:44:00.597 に答える