Please look at the correctness of the class listing, in particular, what is the copy constructor interested in, and how is it possible to do an overload of the operation "=" here?

#include <string.h> #include "Employee.h" Employee::Employee(const char* name) : name_(0) { copyString(&name_, name); } Employee::Employee(const Employee& copy){ name_ = copy.name_; } Employee::~Employee() {} Employee& Employee::operator=(const Employee& copy) { name_ = copy.name_; return *this; } void Employee::copyString(char** dest, const char* source) { size_t str_len = strlen(source); char* str = new char[str_len+1]; strncpy(str, source, str_len); str[str_len] = '\0'; *dest = str; delete[] str; } std::ostream& operator<<(std::ostream& out, const Employee& employee) { out << employee.name_ << std::endl; return out; }

    1 answer 1

    You have incorrectly defined a copy constructor, a copy assignment operator, and a destructor.

    For example, the copy constructor must copy a string, and not just assign one pointer to another. The assignment copy statement must first delete the current name_ pointer before allocating a new memory. And the destructor should release the allocated memory.

    Below is a demonstration program that shows how they can be defined. In addition, the copyString function should be declared as static and private.

     #include <iostream> #include <list> #include <string> #include <cstring> class Employee { private: char *name_; private: static void copyString( char **dest, const char *source ) { *dest = new char[ std::strlen( source ) + 1 ]; strcpy( *dest, source ); } public: Employee( const char *name ) : name_( nullptr ) { copyString( &name_, name ); } Employee( const Employee &copy ) : name_( nullptr ) { copyString( &name_, copy.name_ ); } Employee() { delete [] name_; } Employee & operator =( const Employee &copy ) { if ( this != &copy ) { delete [] name_; name_ = nullptr; copyString( &name_, copy.name_ ); } return *this; } friend std::ostream &operator <<( std::ostream &out, const Employee &employee ) { return out << employee.name_; } }; class Company { public: Company( const std::string &name ) : name( name ) {}; virtual ~Company() = default; void hire( const Employee &employee ) { employees.push_back( employee ); } friend std::ostream & operator <<( std::ostream &out, const Company &company ); private: std::string name; typedef std::list<Employee> EmployeeList; EmployeeList employees; }; std::ostream & operator <<( std::ostream &out, const Company &company ) { out << company.name << "(" << company.employees.size() << "): "; for ( const auto &employee : company.employees ) { out << "\"" << employee << "\" "; } out << std::endl; return out; } void addStuff( Company &company ) { company.hire( Employee( "Max Mustermann" ) ); company.hire( Employee( "Erika Musterfrau" ) ); } int main() { Company company( "OwesomeCo" ); addStuff( company ); std::cout << company; return 0; }

    Output of the program to the console

     OwesomeCo(2): "Max Mustermann" "Erika Musterfrau"

    If you wish, you can also add a relocation constructor and an assignment operator to the Employee class. For example,

     Employee( Employee &&copy ) : name_( nullptr ) { std::swap( name_, copy.name_ ); } Employee & operator =( Employee &&copy ) { if ( this != &copy ) { std::swap( name_, copy.name_ ); } return *this; }
    • Thank you so much for me to do it without you, fix everything and everything works. Thanks !!! - Anton Barinov
    • @AntonBarinov You can safely throw away name_ = nullptr; in the assignment operator - this is clearly superfluous, because after deleting name_ , a new pointer is immediately assigned ... - Harry
    • 2
      @Harry Exception may occur during assignment. Then the state of the object will be undefined. Assigning nullptr makes the state of the object defined regardless of the exception. - Vlad from Moscow
    • A relocation constructor wouldn't hurt, considering that Employee is stored in a container. - Pavel Mayorov
    • @VladfromMoscow Then you must first create a new line, and only then delete the old one - this will provide a more stringent security guarantee. By the way, according to Satter, the only thing you can really do with bad_alloc is to display a message and terminate the program. - Harry