C++中使用new关键字的构造函数

Question

我犯了一个非常愚蠢的错误，就是在一个简单的类中包装一个指向一些新分配的内存的指针。

class Matrix
{
  public:
    Matrix(int w,int h) : width(w),height(h)
    {           
        data = new unsigned char[width*height];
    }

    ~Matrix() { delete data;    }

    Matrix& Matrix::operator=(const Matrix&p)
    {  
            width = p.width;
            height = p.height;
            data= p.data;
            return *this;
    }
    int width,height;
    unsigned char *data;
}

.........
// main code
std::vector<Matrix> some_data;

for (int i=0;i<N;i++) {
   some_data.push_back(Matrix(100,100)); // all Matrix.data pointers are the same
}

当我使用类的实例填充向量时，内部数据指针最终都指向相同的内存吗？

- Martin Beckett

---

谢谢大家 - 没有什么比在周一站起来向世界宣布“我是个白痴”更好的了 ;-） - Martin Beckett

Answer 1

1. 你缺少复制构造函数。

2. 你的赋值运算符不能只是简单地复制指针，因为这会使得多个Matrix对象共享同一个data指针，这将导致该指针被多次delete。相反，你应该创建矩阵的深拷贝。参见关于复制并交换惯用语的问题，其中@GMan详细解释了如何编写高效、异常安全的operator=函数。

3. 在析构函数中应该使用delete[]而不是delete。

- John Kugelman

---

请注意，然而这个赋值操作符并不是异常安全的——如果你的 data = new unsigned char[width * height]; 失败了，旧数据就会消失，但新数据还没有替换它，所以你的矩阵就不再有效。 - Jerry Coffin

谢谢大家，我写了赋值运算符而不是复制构造函数！我一直在使用Qt为您做这个，时间太长了，以至于我忘记了。 - Martin Beckett

是的，我甚至不需要赋值运算符 - 它被手指记忆写下来了 - 只是不太好！delete [] 只是一个剪切和粘贴，简化了这里的代码。 - Martin Beckett

@Martin：由于你需要一个析构函数，你将需要考虑赋值和拷贝构造。 - sbi

@Jerry 和 @sbi，感谢提醒。我已经用 GMan 的优秀答案链接替换了我的不太理想的实现。 - John Kugelman

Answer 2

每当您编写复制构造函数，复制赋值运算符或析构函数时，应该同时完成这三个操作。这些是“三大法宝”，而前面的规则是“三法则”。

目前，您的复制构造函数没有执行深度复制。我还建议在实现“三大法宝”时使用拷贝并交换惯用语。就目前而言，您的operator=是不正确的。

---

也许这是一个学习练习，但您应该始终为类分配单一职责。现在，您的类有两个职责：管理内存资源和成为一个Matrix。您应该将它们分开，这样您就有了一个处理资源的类和另一个使用该类来使用资源的类。

那个实用类需要实现The Big Three，但用户类实际上不需要实现任何一个，因为隐式生成的类将由实用类正确处理。

当然，这样的类已经存在，比如std::vector。

- GManNickG

---

我以为这是3.5的规则。 - Martin York

@Martin @sbi：哈，我会自由地提到复制和交换QA，但我太害羞了，不敢开始使用自己的术语。 :) 的确，我认为3.5法则在C++03中运作良好，在C++0x中它变成了5法则。（当然，不再有0.5，因为移动语义意味着交换语义。） - GManNickG

有人能给我指点一下关于0.5的问题吗？我不明白这个参考（或者如果这是一个笑话的话）的意思。 - Michael Burr

@Gman - 是的，Matrix()只是一个我正在遇到问题的更复杂的稀疏矩阵库的替代品。这只是一个5分钟的丢弃存根代码，浪费了一个小时 - 特别是因为每个矩阵的测试值都相同，所以我没有注意到它是相同的内存。 - Martin Beckett

@Martin：啊，明白了。我仍然会尝试将稀疏数据存储与稀疏数据操作分开，这样一个类只需要实现资源管理，而另一个类则使用该资源。 - GManNickG

Answer 3

您遗漏了拷贝构造函数。

Matrix(const Matrix& other) : width(other.w),height(other.h)
{           
    data = new unsigned char[width*height];
    std::copy(other.data, other.data + width*height, data); 
}

编辑：您的析构函数有误。您需要使用delete[]而不是delete。此外，您的赋值运算符只是复制已分配数组的地址，而没有进行深拷贝。

- pmr

---

@sbi 我想其他人已经添加了这些信息，觉得没有必要在这里重复。已更改。 - pmr

Answer 4

问题在于您正在创建一个临时对象Matrix(100,100)，该对象在被浅拷贝到向量后被销毁。然后在下一次迭代中，它会再次被构造，并为下一个临时对象分配相同的内存。

要解决这个问题：

some_data.push_back(new Matrix(100,100));

当你完成时，你还需要添加一些代码来删除矩阵中的对象。

编辑：还要修复其他答案中提到的问题。这也很重要。但是，如果您更改副本构造函数和赋值运算符以执行深层复制，则不要在填充向量时使用“new”对象，否则会泄漏内存。

- Amardeep AC9MF

---

我从未意识到这会导致相同的内存被分配。我只需要浅层复制，因为我只复制指针！我从来没有想过析构函数会被调用，因为我从来没有调试过析构函数——因为析构函数不可能失败，对吧？下次面试时，我想把这个问题作为一个面试题。 - Martin Beckett

我不能在向量中进行新的操作，因为这只是一个更复杂的矩阵类的快速存根。但是引用计数对象和智能指针让我变得懒惰了，我已经忘记担心这些事情了。 - Martin Beckett

Answer 5

你缺少的复制构造函数已经被指出。当你修复它时，你仍然会有一个主要问题：你的赋值运算符正在进行浅拷贝，这将导致未定义的行为（删除相同的数据两次）。你需要进行深拷贝（即，在你的operator=中分配新空间，将现有内容复制到新空间），或者使用类似引用计数的东西来确保数据只在最后一个引用被销毁时被删除一次。

编辑：冒着发表评论的风险，你所发布的基本上是为什么应该使用标准容器而不是编写自己的容器的典型例子。如果你想要一个矩形矩阵，请考虑将其编写为向量的包装器。

- Jerry Coffin

Answer 6

你正在使用new[]，但你没有使用delete[]。这是一个非常糟糕的想法。

而且你的赋值运算符使得两个实例引用相同的分配内存 - 这两个实例都会尝试释放它！哦，还有你在赋值过程中泄漏了左侧旧内存。

而且，是的，你也缺少复制构造函数。这就是三大法则的内容。

- sbi