2

最近我从一家外部公司获得了一段代码，这似乎有点奇怪。所以我的问题是：有人能解释一下以下代码设计的优缺点吗？

有一个

class A
{
  double val[2]; // actually within the original class there are: 
  // an int, two std::string,a boost::any(for a double and double*)
  // and a boost::shared_array<int>
  public:
  A(double value1, double value2)
  { 
    val[0] = value1;
    val[1] = value2;
  }
  // with normal constructor stuff here, but also
  A(A* const& a) // a pointer-to-object "copy"-constructor?!
  {
    memcpy(this->val, a->val, sizeof(double) * 2 ); 
    // no delete here either...
  }
}

它用于 std::map 之类的（不是指针映射！）

typedef std::map<std::string, A> aMap;
aMap mapOfA;

然后我在代码中到处都发现了很多这些小宝石：

mapOfA.insert(std::make_pair("elite", new A(1.337)));

特别注意A之前的那个“新”！据我所知，关键字“new”在堆中分配内存，必须再次用 delete 删除（boost 指针和类似的除外），我希望它现在仍然正确。;)

所以代码在堆上创建了一个对象，通过指针将它传递给那个特殊的指针转换复制构造函数，该构造函数又通过值将自己传递给std::make_pair，因为映射想要存储整个对象而不是指针。到目前为止我是否理解这一点？出于好奇，我评论了“复制”-构造函数，它在每个小宝石上都给出了我预期的编译器错误（从 A* 转换为非标量类型 A 请求）。

鉴于它是用 GCC 4.1.2 编译的，是否有任何我不知道的机制使这实际上有用？在我看来，这只是非常糟糕的做法！不仅速度慢，而且内存泄漏很多，不是吗？

编辑：

我更改为 double 数组，因为 ppl 对实际上没问题的 memcpy 功能进行了很多评论，因为它实际上是一个 boost::shared_array 我只是出于示例目的而简化了。

4

3 回答 3

3

鉴于它是用 GCC 4.1.2 编译的，是否有任何我不知道的机制使这实际上有用？

不，这只是糟糕的代码。你可以清理它（你可能应该从删除A(A* const& a)代码开始）。

在我看来，这只是非常糟糕的做法！

它使用它生成的每个元素分配/泄漏一个元素。它还使用 memcpy （显然不是赋值？）。

不仅速度慢，而且内存泄漏很多，不是吗？

是的。

如果我猜想有人在从指针初始化时尝试实现移动构造函数，并把它弄得一团糟（可能在 C++11 之前？）。这只是一个猜测。

于 2013-08-21T12:43:59.053 回答
1

你的理解是正确的，这段代码很疯狂。作者显然不了解 C++ 内存管理，甚至不了解赋值之类的基本概念；这memcpy是一个奇怪的混淆val = a->val;

如果我不得不猜测，我会说作者误解了必须使用创建类对象new（因为它们是在其他流行语言中），并添加了怪异的构造函数来修复随后的编译错误。

正确的解决方法是可能删除new：

mapOfA.insert(std::make_pair("elite", A(1.337)));

或者只是1.337，如果转换 fromdouble应该是隐式的。

于 2013-08-21T12:55:16.453 回答
1

呸！为什么不放弃memcpy并简单地使用：

val = a->val;
于 2013-08-21T12:34:15.027 回答