c++ - 此 C++ 代码包含哪些未定义的行为

我在阅读 Effective C++(第三版)的第 11 条后写下了这段代码。

#include <iostream>
using namespace std;

#define MAX_COLORS 20
class Widget
{
 public:
    Widget ( int seed );
    ~Widget ( );
    Widget& operator=( const Widget& rhs );
    void ToString ( );
 private:

    Widget& SelfAssignmentUnsafe ( const Widget& rhs );
    Widget& SelfAssignmentSafe ( const Widget& rhs );
    Widget& SelfAssignmentAndExceptionSafe ( const Widget& rhs );
    void MakeDeepCopy ( const Widget& rhs );
    int *colorPallete;
};

void Widget::ToString()
{
 int i = 0;
 for ( i = 0; i < MAX_COLORS; i++ )
 {
 cout << "colorPallete[" << i << "]: " << colorPallete[i] << endl;
 }
}

Widget::Widget ( int seed ):
    colorPallete ( new int[MAX_COLORS])
    {
     int i = 0;
     for ( i = 0; i < MAX_COLORS; i++ )
     {
      colorPallete[i] = seed + i;
     }
    }

Widget& Widget::operator=( const Widget& rhs )
{
//    return SelfAssignmentUnsafe ( rhs );

//    return SelfAssignmentSafe( rhs );

    return SelfAssignmentAndExceptionSafe ( rhs );
}

Widget& Widget::SelfAssignmentUnsafe ( const Widget& rhs )
{
    delete[] colorPallete;
    colorPallete = 0;
    MakeDeepCopy( rhs );
    return *this;
}

Widget& Widget::SelfAssignmentSafe ( const Widget& rhs )
{
    if ( this == &rhs ) return *this;

    delete[] colorPallete;
    colorPallete = 0;
    MakeDeepCopy ( rhs );
    return *this;
}

void Widget::MakeDeepCopy ( const Widget& rhs )
{
    int i = 0;
    colorPallete = new int [MAX_COLORS];
    for ( i = 0;i < MAX_COLORS; i++ )
    {
     colorPallete[i] = rhs.colorPallete[i];
    }
}

Widget& Widget::SelfAssignmentAndExceptionSafe ( const Widget& rhs )
{
    int *origColorPallete = colorPallete;
    MakeDeepCopy ( rhs );
    delete[] origColorPallete;
    origColorPallete = 0;
    return *this;
}

Widget::~Widget()
{
 delete[] colorPallete;
}


int main()
{
 Widget b(10);
 Widget a(20);
 b.ToString();
 b = b;
 cout << endl << "After: " << endl;
 b.ToString();
}

作者讲了在赋值运算符中处理对self的赋值:

Widget a(10);
a = a;

从 Widget 的赋值运算符我调用 Widget::SelfAssignmentAndExceptionSafe。

在 Widget::SelfAssignmentAndExceptionSafe 中，想法是将 colorPallete 指针保存在 origColorPallete 中。然后制作一份 rhs.colorPallete 的深层拷贝。当复制成功时，我删除原始指针并返回对 self 的引用。

上述机制应该是自分配和异常安全的。

但是， Widget::SelfAssignmentAndExceptionSafe 无法正确处理对 self 的赋值。 colorPallete 数组在自赋值后包含垃圾。它很好地处理了其他情况。

为什么会这样？

请帮忙。

[编辑:检查所有答案后]

感谢您的回答。我已经更新了 MakeDeepCopy 方法并且该示例现在工作正常。下面，我粘贴了更新后的代码:

#include <iostream>

using namespace std;

#define MAX_COLORS 20
class Widget
{
 public:
    Widget ( int seed );
    ~Widget ( );
    Widget& operator=( const Widget& rhs );
    void ToString ( );
 private:
    Widget( Widget& rhs );
    Widget& SelfAssignmentUnsafe ( const Widget& rhs );
    Widget& SelfAssignmentSafe ( const Widget& rhs );
    Widget& SelfAssignmentAndExceptionSafe ( const Widget& rhs );
    void MakeDeepCopy ( const int* rhs );
    int *colorPallete;
};

void Widget::ToString()
{
 int i = 0;
 for ( i = 0; i < MAX_COLORS; i++ )
 {
 cout << "colorPallete[" << i << "]: " << colorPallete[i] << endl;
 }
}

Widget::Widget ( int seed ):
    colorPallete ( new int[MAX_COLORS])
    {
     int i = 0;
     for ( i = 0; i < MAX_COLORS; i++ )
     {
      colorPallete[i] = seed + i;
     }
    }

Widget& Widget::operator=( const Widget& rhs )
{
//    return SelfAssignmentUnsafe ( rhs );

//    return SelfAssignmentSafe( rhs );

    return SelfAssignmentAndExceptionSafe ( rhs );
}

Widget& Widget::SelfAssignmentUnsafe ( const Widget& rhs )
{
    delete[] colorPallete;
    colorPallete = 0;
    MakeDeepCopy( rhs.colorPallete );
    return *this;
}

Widget& Widget::SelfAssignmentSafe ( const Widget& rhs )
{
    if ( this == &rhs ) return *this;

    delete[] colorPallete;
    colorPallete = 0;
    MakeDeepCopy ( rhs.colorPallete );
    return *this;
}

void Widget::MakeDeepCopy ( const int* rhs )
{
    int i = 0;
    colorPallete = new int [MAX_COLORS];
    for ( i = 0;i < MAX_COLORS; i++ )
    {
     colorPallete[i] = rhs[i];
    }
}

Widget& Widget::SelfAssignmentAndExceptionSafe ( const Widget& rhs )
{
    int *origColorPallete = colorPallete;
    MakeDeepCopy ( rhs.colorPallete );
    delete[] origColorPallete;
    origColorPallete = 0;
    return *this;
}

Widget::~Widget()
{
 delete[] colorPallete;
}


int main()
{
 Widget b(10);
 Widget a(20);
 b.ToString();
 b = b;
 cout << endl << "After: " << endl;
 b.ToString();
}

[编辑:根据查尔斯的回应修改代码]

这个想法是实现“ copy-and-swap ”习语，使代码既自赋值又安全异常。请注意，复制仅在复制构造函数中实现。如果复制成功，我们交换赋值运算符。

与之前更新相比的另一个改进是 MakeDeepCopy 的界面取决于正确使用。我们必须在调用 MakeDeepCopy 之前存储/删除 colorPallete 指针。现在不存在这样的依赖关系。

#include <iostream>

using namespace std;

#define MAX_COLORS 20
class Widget
{
 public:
    Widget ( int seed );
    ~Widget ( );
    Widget& operator=( const Widget& rhs );
    void ToString ( );
    Widget( const Widget& rhs );
 private:
    int *colorPallete;
};

void Widget::ToString()
{
 int i = 0;
 for ( i = 0; i < MAX_COLORS; i++ )
 {
 cout << "colorPallete[" << i << "]: " << colorPallete[i] << endl;
 }
}

Widget::Widget ( int seed ):
    colorPallete ( new int[MAX_COLORS])
    {
     int i = 0;
     for ( i = 0; i < MAX_COLORS; i++ )
     {
      colorPallete[i] = seed + i;
     }
    }

Widget::Widget( const Widget& rhs ):
    colorPallete( new int[MAX_COLORS] )
{
    std::copy ( rhs.colorPallete, rhs.colorPallete + MAX_COLORS, colorPallete );
}

Widget& Widget::operator=( const Widget& rhs )
{
    Widget tmp(rhs);

    std::swap ( colorPallete, tmp.colorPallete );

    return *this;
}

Widget::~Widget()
{
 delete[] colorPallete;
}


int main()
{
 Widget b(10);
 Widget a(20);
 b.ToString();
 b = b;
 cout << endl << "After: " << endl;
 b.ToString();
}

最佳答案

您看到的垃圾是因为 MakeDeepCopy 函数总是从 colorPallete 的 rhs 成员复制，而不是您在 origColorPallete 中制作的拷贝。

以下修改将修复它:

int *Widget::MakeDeepCopy ( const int *rhs )
{
    int i = 0;
    int *colorPallete = new int [MAX_COLORS];
    for ( i = 0;i < MAX_COLORS; i++ )
    {
     colorPallete[i] = rhs[i];
    }
    return colorPallete;
}

Widget& Widget::SelfAssignmentAndExceptionSafe ( const Widget& rhs )
{
    int *origColorPallete = colorPallete;
    colorPallete = MakeDeepCopy ( origColorPallete );
    delete[] origColorPallete;
    origColorPallete = 0;
    return *this;
}

实际上，通过上述修改，您可能希望将 MakeDeepCopy 重命名为 CopyColorPalette 或其他名称(特别是如果您想保留原始 MakeDeepCopy 用于其他目的)。

关于c++ - 此 C++ 代码包含哪些未定义的行为，我们在Stack Overflow上找到一个类似的问题：https://stackoverflow.com/questions/1187687/