5

以下赋值和复制移动构造函数是最有效的吗?如果有人有其他方法请告诉我?我的意思是 std::swap 怎么样?在下面的代码中通过复制构造函数调用赋值是安全的吗?

#include <iostream>
#include <functional>
#include <algorithm>
#include <utility>

using std::cout;
using std::cin;
using std::endl;
using std::bind;


class Widget
{

public:

    Widget(int length)
        :length_(length),
        data_(new int[length])
    {
        cout<<__FUNCTION__<<"("<<length<<")"<<endl;
    }

    ~Widget()
    {
        cout<<endl<<__FUNCTION__<<"()"<<endl;
        if (data_)
        {
            cout<<"deleting source"<<endl;
        } 
        else
        {
            cout<<"deleting Moved object"<<endl;
        }

        cout<<endl<<endl;
    }

    Widget(const Widget& other)
        :length_(other.length_),
        data_(new int[length_])
    {
        cout<<__FUNCTION__<<"(const Widget& other)"<<endl;
        std::copy(other.data_,other.data_ + length_,data_);
    }

    Widget(Widget&& other)
/*
        :length_(other.length_),
        data_(new int[length_])*/
    {
        cout<<__FUNCTION__<<"(Widget&& other)"<<endl;
        length_ = 0;
        data_ = nullptr;
        std::swap(length_,other.length_);
        std::swap(data_,other.data_);
    }

    Widget& operator = (Widget&& other)
    {
        cout<<__FUNCTION__<<"(Widget&& other)"<<endl;

        std::swap(length_,other.length_);
        std::swap(data_,other.data_);

        return *this;
    }

    Widget& operator = (const Widget& other)
    {
        cout<<__FUNCTION__<<"(const Widget& other)"<<endl;
        Widget tem(other);
        std::swap(length_,tem.length_);
        std::swap(data_,tem.data_);

        return *this;
    }
    int length()
    {
        return length_;
    }

private:

    int length_;
    int* data_;
};



int main()
{
    {
        Widget w1(1);
        Widget w2(std::move(Widget(2)));

        w1 = std::move(w2);
    }


    cout<<"ENTER"<<endl;
    cin.get();
    return 0;
}
4

3 回答 3

7

从效率 POV 看起来不错,但包含大量重复的代码。ID

  • swap()为您的类实现一个运算符。
  • 初始化length_data_声明它们的位置。
  • 尽可能根据其他操作实施操作。

可能想要使用std::memcpy,而不是std::copy因为您正在处理原始数组。一些编译器会为你做到这一点,但可能不是所有的编译器......

这是您的代码的重复数据删除版本。注意只有一个地方需要知道两个实例是如何Widget交换的。只有一个地方知道如何分配给定大小的小部件。

编辑:您通常还想使用依赖于参数的查找来定位交换,以防万一您有非原始成员。

编辑:集成@Philipp 建议让赋值运算符按值接受它的参数。这样,它既充当移动赋值运算符,又充当复制赋值运算符。在移动的情况下,如果你传递一个临时的,它不会被复制,因为移动构造函数,而不是复制构造函数将用于传递参数。

编辑: C++11 允许在右值上调用非成本成员,以便与标准的先前版本兼容。这允许编译奇怪的代码Widget(...) = someWidget。通过在声明之后放置需要operator=一个左值来防止这种情况发生。请注意,即使没有该限制,代码也是正确的,但这似乎是个好主意,所以我添加了它。this&

编辑:正如 Guillaume Papin 指出的那样,析构函数应该使用delete[]而不是 plain delete。C++ 标准要求通过new []删除分配的内存delete [],即它允许new' andnew []` 使用不同的堆。

class Widget
{
public:
    Widget(int length)
        :length_(length)
        ,data_(new int[length])
    {}

    ~Widget()
    {
        delete[] data_;
    }

    Widget(const Widget& other)
        :Widget(other.length_)
    {
        std::copy(other.data_, other.data_ + length_, data_);
    }

    Widget(Widget&& other)
    {
        swap(*this, other);
    }

    Widget& operator= (Widget other) &
    {
        swap(*this, other);
        return *this;
    }

    int length() const
    {
        return length_;
    }

private:
    friend void swap(Widget& a, Widget& b);

    int length_ = 0;
    int* data_ = nullptr;
};

void swap(Widget& a, Widget& b) {
    using std::swap;
    swap(a.length_, b.length_);
    swap(a.data_, b.data_);
}
于 2012-09-29T14:24:10.660 回答
0

答案是针对@Abdulrhman 在上述评论中的抱怨,即某些(模糊的)任务序列失败了。放入单独的答案,因为这样更具可读性。

投诉是这样的

Widget w(2);
w = Widget(1) = std::move(w);

崩溃。这是我得到的输出

Widget w(2);
w.data()[0] = 0xDEAD; w.data()[1] = 0xBEEF;
w = Widget(1) = std::move(w);
std::cerr << std::hex << w.data()[0] << w.data()[1] << std::endl;

添加了一些代码Widget来记录构造函数、析构函数和赋值运算符的调用。交错是关于这些呼叫来自何处的评论

w is constructed
0x7fff619c36c0: [constructor] allocated 2@0x1043dff80
temporary Widget(1) is constructed
0x7fff619c37c0: [constructor] allocated 1@0x1043e0180
first (right) assignment operator argument is constructed. w is empty afterwards!
0x7fff619c3800: [default constructor] empty
0x7fff619c3800: [move constructor] stealing 2@0x1043dff80 from 0x7fff619c36c0, replacing with 0@0x0
first assignment operator does it's job, i.e. moves from by-value argument.
0x7fff619c37c0: [assignment] stealing 2@0x1043dff80 from 0x7fff619c3800, replacing with 1@0x1043e0180
second (left) assignment operator arguments is constructed
0x7fff619c3780: [constructor] allocated 2@0x1043e0280
0x7fff619c3780: [copy constructor] copying 2@0x1043dff80 from 0x7fff619c37c0
second assignment operator does it's job, i.e. moves from by-value argument
0x7fff619c36c0: [assignment] stealing 2@0x1043e0280 from 0x7fff619c3780, replacing with 0@0x0
second assingment operator's by-value argument is destructed
0x7fff619c3780: [destructor] deleting 0@0x0
first assignment operator's by-value argument is destructed
0x7fff619c3800: [destructor] deleting 1@0x1043e0180
temporary created as Widget(1) is destructed.
0x7fff619c37c0: [destructor] deleting 2@0x1043dff80
data contains in "w" after assignments.
deadbeef
finally, "w" is destructed.
0x7fff619c36c0: [destructor] deleting 2@0x1043e0280

我在那里看不出问题,并用clang编译它,-faddress-sanitizer, -fcatch-undefined-behaviour也没有抱怨。

但请注意,第二个分配(左=运算符)复制而不是移动。这是因为第一个(右)赋值运算符返回一个左值引用。

于 2012-10-01T10:41:35.897 回答
0

您不需要在移动构造函数中进行如此多的交换和分配。这:

Widget(Widget&& other) :
    length( other.length_ ), data( other.data_ )
{
    other.length_ = 0;
    other.data_ = nullptr;
}

为移动构造函数做最少的工作:总共 4 个分配。您的版本有 8 个,计算对 swap() 的调用中的那些。

你的移动分配没问题,但你可能想考虑只写一个 operator=() 来涵盖这两种情况:

Widget &operator=( Widget other ) {
    delete data_;
    data_ = other.data_;
    other.data_ = nullptr;
    length_ = other.length_;
    other.length_ = 0;
    return *this;
}

这比您的版本效率略低,因为它可以移动两次。

于 2012-10-01T13:25:08.023 回答