 <p>As a1ex07 says in a comment, your code looks more like what you would put in <code>operator=</code> than in a copy constructor.</p> <p>First of all, a copy constructor is initializing a brand new object. A check like <code>if (this != &amp;p)</code> doesn't make much sense in a copy constructor; the point you are passing to the copy constructor is never going to be the object that you are initializing at that point (since it's a brand new object), so the check is always going to be <code>true</code>.</p> <p>Also, doing things like <code>if (label != NULL)</code> is not going to work, because <code>label</code> is not yet initialized. This check might return <code>true</code> or <code>false</code> in random ways (depending if the uninitialized <code>label</code> is <code>NULL</code> by chance).</p> <p>I would write it like this:</p> <pre><code>GPSPoint(const GPSPoint&amp; 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; } } </code></pre> <p>Maybe making <code>label</code> a <code>std::string</code> instead of a C-style <code>char*</code> would be a better idea, then you could write your copy constructor purely using an initializer list:</p> <pre><code>class GPSPoint { private: double lat, lon, h; std::string label; public: // Copy constructor GPSPoint(const GPSPoint&amp; p) : lat(p.lat), lon(p.lon), h(p.h), label(p.label) { } } </code></pre>