Идиома Copy-and-Swap в С++

Уже не в первый раз на моем аккаунте затрагивается Вопрос про задачу о ломаной. На этот раз я хотел бы обсудить "Copy-and-Swap Idiom in C++" в моем коде. Условие задачи можете прочитать тут, если интересно.

Значит, в своем коде я реализовал идиому Copy-and-Swap:

void swap(Loman& a) {
        std::swap(size, a.size);
        std::swap(data, a.data);
    }
 
    Loman& operator=(const Loman& a)
    {
            Loman tmp(a);
            swap(tmp);
        return *this;
    }

Но мне кажется в этом коде есть логиеская опечатка и она должна, на самом деле, записываться так:

void swap(Loman& first, Loman& second) {
        std::swap(first.size, second.size);
        std::swap(first.data, second.data);
    }
 
    Loman& operator=(const Loman& a)
    {
            Loman tmp(a);
            swap(*this, tmp);
        return *this;
    }

Так же мне кажется, что в операторе + есть ненужная операция this-> size = size. Если уберу его, то ничего не изменится.

Вот весь код:

#include <iostream>
using namespace std;
class Loman
{
private:
    int i=0;
    int size;
    int* data;
public:
    Loman(int asize) {
        size = 2*asize;
        data = new int[size];
    }
    ~Loman(){
        delete[] data;
        cout<<"Object destroyed\n";
    }
    Loman(const Loman &ob)
    {
        size = ob.size;
        data = new int [size];
        for(int i=0; i<ob.size; i++)
            data[i] = ob.data[i];
    }
    void print(){
        for(i=0; i<size; i++){
            cout<<*(data+i)<<" ";
        }
        cout<<endl;
    }
 
    void scan(){
        cout<<"Input "<<size/2<<" coordinates\n";
        for(int i=0; i<size; i++){
            cin>>*(data+i);
        }
    }
    Loman operator+(Loman& ob){
        this->size = size;
        Loman tmp((size + ob.size)/2);
        for(i=0; i<size; i++)
        {
            tmp.data[i]=*(data + i);//this->data[i]=data[i];
        }
        for(i=size; i<size + ob.size; i++) tmp.data[i] = ob.data[i-size];
        return tmp;
    }
    void swap(Loman& a) {
        std::swap(size, a.size);
        std::swap(data, a.data);
    }
 
    Loman& operator=(const Loman& a)
    {
            Loman tmp(a);
            swap(tmp);
        return *this;
    }
};
int main(){
    Loman first(3);
    Loman second(2);
    first.scan();
    second.scan();
    Loman third(5);
    third=first+second;
    third.print();
    return 0;
}

Вопросы: Верны ли мои гепотезы про оператор "+", а также про функцию swap, если нет почему?
Упустил ли я еще какие-нибудь ошибки?
Необходимо ли в функции swap написать using std::swap;


Ответы (1 шт):

Автор решения: HolyBlackCat

Начинаю с конца.

Необходимо ли в функции swap написать using std::swap;

Здесь не обязательно. using std::swap;, а затем swap(x, y); (уже без std::) - это канонический вызов swap на произвольном типе.

По умолчанию он вызовет std::swap, но если для этого типа есть своя функция swap (там, где ее найдет ADL - например, в том же namespace), то будет вызвана она.

Но поскольку здесь вы ее вызываете только на встроенных типах (не на классах), никакого ADL все равно не будет, и смысла заморачиваться нет.


Отсюда вытекает ответ про вашу функцию swap. Если вы хотите, чтобы ее можно было вызвать таким образом, то она не должна быть нестатическим членом. (Потому что мы делаем swap(x, y);, а не x.swap(y);.)

То, что вы написали: void swap(Loman &first, Loman &second) - это совсем мимо, потому что такая функция принимает три объекта, а не два (третий - this, получается x.swap(y, z)).

Правильно: friend void swap(Loman& first, Loman& second) {...}. Можно было бы подумать, что static подойдет, но нет, ADL его не уважает.


Еще стоит дописать конструктор перемещения (noexcept не забудьте). После этого operator= лучше переписать так:

Loman& operator=(Loman a) noexcept
{
    swap(*this, a);
    return *this;
}

Тогда он будет подходить одновременно и для копирования, и для перемещения. Я бы использовал такую форму в любом случае (даже если нет либо копирующего конструктора, либо перемещающего), чтобы не думать.


Loman operator+(Loman& ob) нужно заменить на Loman operator+(const Loman &) const, чтобы + мог принимать константные и/или временные объекты.

Аналогично, void print() const.


В operator+, this->size = size; ничего не делает. Это одна и та же переменная.


Смущает привычка везде писать *(data+i). Чем не угодил data[i]?

Еще смущает идея хранить X и Y вперемешку в одном массиве. Лучше сделайте себе struct Point {int x, y;} и храните в массиве его. Тогда не нужно будет заморачиваться с умножением размера на два.

→ Ссылка