2016-12-26 3 views
0

Я написал структуру Point, которую я использую для моделирования проблемы n-тела. Мне было трудно полностью понять и реализовать копию подкачки и адаптировать ее к моим потребностям, которые в основном связаны с скоростью. Правильно ли я делаю это? Будет ли другое в C++ 17?C++ & Swap/Copy применяется к точке struct

#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

Вам нужно удалить ВСЕ указатели и ВСЕ звонки в 'новый' из вашего кода. Также измените ВСЕ нормальные арифметические операторы (+, не + =) для возврата по значению. –

+2

Вы 'operator =' функция меняет объекты на * обеим сторонам оператора присваивания. Он также не может использоваться с r значениями в правой части оператора (что означает «r» в «rvalue»). Оператор должен принимать аргумент * по значению *, если вы используете swap, а по * константе * ссылку, если нет. –

ответ

4

Копия/своп-ideom фактически копирует иswap() сек значения. Ваша «адаптация» всего лишь swap()s. Правильное использование копирования/своп-ideom будет выглядеть, например, так:

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

(конечно, это также предполагает, что ваша swap() функция является членом принимает только один дополнительный аргумент: там является уже объект для обмена с).

Если скорость является вашей основной задачей, то копия/swap-идеома, вероятно, не подходит для случая 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; 
} 

Как было отмечено в комментариях, вы должны также не выделять новые Point объекты с new: 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