2016-12-26 3 views
0

n体の問題をモデル化するために使用しているPoint構造体を記述しました。私は、コピー&のスワップイディオムを完全に理解して実装し、それを主に速度である私のニーズに適合させるのは難しいと感じました。私はこれを正しくしていますか?それはC++ 17では違うでしょうか?ポイント構造体にC++とスワップ/コピーを適用

#pragma once 
#include <algorithm> 

struct Point 
{ 
    double x, y, z; 

    explicit Point(double X = 0, double Y = 0, double Z = 0) : x(X), y(Y), z(Z) {} 
    void swap(Point&, Point&); 

    inline bool operator==(Point b) const { return (x == b.x && y == b.y && z == b.z); } 
    inline bool operator!=(Point b) const { return (x != b.x || y != b.y || z != b.z); } 

    Point& operator=(Point&); 
    Point& operator+(Point&) const; 
    Point& operator-(Point&) const; 
    inline double operator*(Point& b) const { return b.x*x + b.y*y + b.z*z; } // Dot product 
    Point& operator%(Point&) const; // % = Cross product 

    inline Point& operator+=(Point& b) { return *this = *this + b; } 
    inline Point& operator-=(Point& b) { return *this = *this - b; } 
    inline Point& operator%=(Point& b) { return *this = *this % b; } 

    Point& operator*(double) const; 
    Point& operator/(double) const; 

    inline Point& operator*=(double k) { return *this = *this * k; } 
    inline Point& operator/=(double k) { return *this = *this/k; } 
}; 

std::ostream &operator<<(std::ostream &os, const Point& a) { 
    os << "(" << a.x << ", " << a.y << ", " << a.z << ")"; 
    return os; 
} 

void Point::swap(Point& a, Point& b) { 
    std::swap(a.x, b.x); 
    std::swap(a.y, b.y); 
    std::swap(a.z, b.z); 
} 

Point& Point::operator=(Point& b) { 
    swap(*this, b); 
    return *this; 
} 

Point& Point::operator+(Point& b) const { 
    Point *p = new Point(x + b.x, y + b.y, z + b.z); 
    return *p; 
} 

Point& Point::operator-(Point& b) const { 
    Point *p = new Point(x - b.x, y - b.y, z - b.z); 
    return *p; 
} 

Point& Point::operator%(Point& b) const { 
    Point *p = new Point(
     y*b.z - z*b.y, 
     z*b.x - x*b.z, 
     x*b.y - y*b.x 
); 

    return *p; 
} 

Point& Point::operator*(double k) const { 
    Point *p = new Point(k*x, k*y, k*z); 
    return *p; 
} 

Point& Point::operator/(double k) const { 
    Point *p = new Point(x/k, y/k, z/k); 
    return *p; 
} 
+5

)コードの漏れが篩のようなメモリのように見えます。 –

+0

コードからすべてのポインタとすべての 'new'呼び出しを削除する必要があります。また、すべての通常の算術演算子(+、not + =)を値で戻すように変更します。 –

+2

'operator ='関数は、代入演算子の*両側のオブジェクトを変更します。また、演算子の右辺の値( "rvalue"の "r"が表すもの)と併用することはできません。演算子は、スワップを使用する場合は値*で、そうでない場合は* constant *参照で引数*を取る必要があります。 –

答えて

4

コピー/スワップideomは、実際にコピーswap()の値。あなたの「適応」は単にswap()sです。コピー/スワップideomの正しい使用は、このように、例えば、になります。

Point& Point::operator= (Point other) { // note: by value, i.e., already copied 
    this->swap(other); 
    return *this; 
} 

(もちろん、これはまた、あなたのswap()機能が一つだけ追加の引数を取るメンバーであることを前提としています。すでにがありますスワップするオブジェクト)。

speedが主な関心事である場合、コピー/スワップideomはおそらくPointの場合には特に適していません。コピー操作は本質的に些細なことです。値をスワップすることは、スワップ操作がたぶん複数の値といくつかの割り当て操作をコピーするだけでなく、いくつかのポインタスワップに相当するstd::vectorで古い配列をコピーするような比較的複雑な操作に比べてかなり妥当です。それはあなたのPoint割り当ては単に、すべてのメンバーを割り当てるにオフおそらく最高で、次のとおりです。としては、コメントで指摘された

Point& Point::operator= (Point const& other) { // note: no copy... 
    this->x = other.x; 
    this->y = other.y; 
    this->z = other.z; 
    return *this; 
} 

、あなたもないnewで新しいPointオブジェクトを割り当てる必要があります:C++はJavaやC#ではありません!スタック上に、ヒープから来る必要のないオブジェクトを作成するだけです(例:

Point Point::operator+ (Point const& other) const { 
    return Point(this->x + other.x, this->y + other.y, this->z + other.z); 
} 
+0

詳細な回答ありがとう、他のコメントありがとうございます。私はそれに応じて私のコードを編集しました。 – Sebastian