c++ - Copy constructor for point -

c++ - Copy constructor for point -

is re-create constructor correct?

class gpspoint { private: double lat, lon, h; char *label; public: gpspoint (const gpspoint &p) { if (this != &p) { lat = p.lat; lon = p.lon; h = p.h; if ( label != null ) { delete [] label; label = null; } if (p.label != null ) { label = new char [ strlen( p.label ) + 1 ]; strcpy ( label, p.label ); } } } }

if have pointer in class doing wrong.

it improve re-write as:

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) {} // should have assignment operator gpspoint& operator=(gpspoint copy) // utilize copy/swap idum. { // pass value implicit re-create this->swap(copy); // swap homecoming *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); } };

now looks lot simpler.

but hey forgot there compiler generated re-create constructors: simplifies too:

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

done. simpler better.

if must absolutely maintain pointer (because think optimization (its not), or need larn pointers (you do, need larn when not utilize them)).

class gpspoint { private: double lat; double lon; double h; char* label; public: // don't forget constructor , destructor should initialize label // note below code assumes label never null , // c-string null terminated (but each re-create creates // , maintains 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); } // should have assignment operator gpspoint& operator=(gpspoint copy) // utilize copy/swap idum. { // pass value implicit re-create this->swap(copy); // swap homecoming *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); } };

c++ constructor copy point