点的复制构造函数

如果你在你的类中有一个指针，你可能做错了什么

最好重写为:

class GPSPoint
{
   private:
      double      lat;
      double      lon;
      double      h;
      std::string label;
   public:
    GPSPoint (GPSPoint const& copy)
       : lat(copy.lat)
       , lon(copy.lon)
       , h(copy.h)
       , label(copy.label)
    {}
    // You should also have an assignment operator
    GPSPoint& operator=(GPSPoint copy)    // Use Copy/Swap idum.
    {                                     // Pass by value to get implicit copy
         this->swap(copy);                // Now swap
         return *this;
    }
    void swap(GPSPoint& copy) throw()
    {
        std::swap(lat,  copy.lat);
        std::swap(lon,  copy.lon);
        std::swap(h,    copy.h);
        std::swap(label,copy.label);
    }
};

现在看起来简单多了。

但是我们忘了还有编译器生成的复制构造函数:
所以它现在也简化了:

class GPSPoint
{
   private:
      double      lat;
      double      lon;
      double      h;
      std::string label;
};

。

如果你必须绝对保留指针(因为你认为这是一个优化(它不是)，或者你需要学习指针(你做，但你需要学习什么时候不使用它们)。

class GPSPoint
{
   private:
      double      lat;
      double      lon;
      double      h;
      char*       label;
   public:
   // Don't forget the constructor and destructor should initialize label
   // Note the below code assumes that label is never NULL and always is a
   // C-string that is NULL terminated (but that each copy creates 
   // and maintains its own version)  
   GPSPoint (GPSPoint const& copy)
       : lat(copy.lat)
       , lon(copy.lon)
       , h(copy.h)
       , label(new char[strlen(copy.label)+1])
    {
       std::copy(copy.label, copy.label + strlen(label) + 1, label);
    }
    // You should also have an assignment operator
    GPSPoint& operator=(GPSPoint copy)    // Use Copy/Swap idum.
    {                                     // Pass by value to get implicit copy
         this->swap(copy);                // Now swap
         return *this;
    }
    void swap(GPSPoint& copy) throw()
    {
        std::swap(lat,  copy.lat);
        std::swap(lon,  copy.lon);
        std::swap(h,    copy.h);
        std::swap(label,copy.label);
    }
};

这有点啰嗦;我会重新设计它的样式。

更重要的是，它看起来更像一个op=而不是一个复制构造函数，特别是因为您测试label是否为NULL，就好像它已经可以使用一样。而且，由于它没有初始化，它不太可能已经是NULL了……delete [] label是一个严重错误。

但是如果你把这个作为你的op=，那么我想这在语义上是正确的。

所以不要忘记你的构造函数(并将label初始化为NULL !)，复制构造函数(让它使用op=)和析构函数。

我知道你在之前的问题中说过(没有任何令人信服的理由)你"不想使用std::string"，但这是一个很好的例子，说明为什么你真的应该使用。

在我的理解中，您创建了operator =的有效实现，而不是复制构造函数。如果对象已经存在，则if (this != &p)有意义;对于删除标签

简而言之:不。这不是一个可怕的赋值操作符，但它作为复制构造函数是坏的。

首先，不可能发生自赋值，因为您没有赋值任何东西。this指向构造函数开始时未初始化的内存blob，而p是正在复制的完全构造的Point对象。这两者不能同时发生。因此，初始检查是浪费时间。

再往下，检查确保label不为空。正如我所说，this指向未初始化的内存…此时label可以是任何值，不知道测试是否会通过或失败。如果它不产生空，你将delete[]一个随机的内存部分。如果幸运的话，这将立即失败，但它不必如此。

就风格而言，首选构造函数初始化列表来初始化成员。

GPSPoint (const GPSPoint& copy)
  : lat(copy.lat)
  , lon(copy.lon)
  , h(copy.h)
  , label(0)
 {
   if(copy.label) {
       label = new char[ strlen( copy.label ) + 1 ];
       strcpy ( label, copy.label );
   } 
 } 

在正确性方面，去掉c-string，使用合适的string类。这样，你就根本不需要实现复制构造函数了。

无论如何，如果要实现复制构造函数，请确保实现复制赋值操作符和析构函数;我想这些是为了简洁而省略的，但如果不是的话，它们是非常重要的。

正如a1ex07在注释中所说，您的代码看起来更像是放在operator=中的代码，而不是放在复制构造函数中。

首先，复制构造函数初始化了一个全新的对象。像if (this != &p)这样的检查在复制构造函数中没有多大意义;你传递给复制构造函数的点永远不会是你在那一点初始化的对象(因为它是一个全新的对象)，所以检查总是true。

同样，做if (label != NULL)这样的事情是行不通的，因为label还没有初始化。此检查可能以随机方式返回true或false(取决于未初始化的label是否碰巧是NULL)。

我会这样写:

GPSPoint(const GPSPoint& p) : lat(p.lat), lon(p.lon), h(p.h) {
    if (p.label != NULL) {
        label = new char[strlen(p.label) + 1];
        strcpy(label, p.label);
    }
    else {
        label = NULL;
    }
}

也许让label成为std::string而不是c风格的char*会是一个更好的主意，然后你可以纯粹使用初始化列表来编写复制构造函数:

class GPSPoint {
private:
    double lat, lon, h;
    std::string label;
public:
    // Copy constructor
    GPSPoint(const GPSPoint& p) : lat(p.lat), lon(p.lon), h(p.h), label(p.label) { }
}

是的——我最初读错了。

但是，我仍然建议您使用std::string作为标签，因为它将为您管理内存，并且复制构造函数变得更简单(实际上，在这种情况下是不必要的)。