2

Прошу посмотреть на правильность листинг класса, а именно интересует конструктор копирования, а также как здесь правильно сделать перегрузка операции "="?

#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 ответ 1

4

У вас неправильно определены конструктор копирования, копирующий оператор присваивания и деструктор.

Например, конструктор копирования должен копировать строку, а не просто присваивать один указатель другому. Копирующий оператор присваивания должен сначала удалить текущий указатель name_, прежде чем выделять новую память. А деструктор должен высвобождать выделенную память.

Ниже приведена демонстрационная программа, в которой показано, как они могут быть определены. Кроме того функцию copyString следует объявить как статическую и закрытую.

#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;
}

Вывод программы на консоль

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

При желании вы можете добавить в класс Employee также перемещающий конструктор и перемещающий оператор присваивания. Например,

Employee( Employee &&copy ) : name_( nullptr )
{
    std::swap( name_, copy.name_ );
}

Employee & operator =( Employee &&copy )
{
    if ( this != &copy )
    {
        std::swap( name_, copy.name_ );
    }           

    return *this;
}       
6
  • Спасибо большое, чтобы бы я без вас делал, всё исправил и всё работает. Благодарю !!! Commented 13 окт. 2016 в 22:39
  • @AntonBarinov Можете смело выбросить name_ = nullptr; в операторе присваивания - это явно лишнее, так как после удаления name_ сразу присваивается новый указатель...
    – Harry
    Commented 14 окт. 2016 в 2:52
  • 2
    @Harry Может возникнуть исключение при присваивании. Тогда состояние объекта будет неопределенным. Присваивание nullptr делает состояние объекта определенным независимо от исключения. Commented 14 окт. 2016 в 2:54
  • Конструктор перемещения не помешал бы, учитывая что Employee хранятся в контейнере. Commented 14 окт. 2016 в 6:05
  • @VladfromMoscow Тогда надо сначала создать новую строку, а уже потом удалять старую - это обеспечит более строгую гарантию безопасности. Кстати, если верить Саттеру, то единственное, что можно реально сделать при bad_alloc - это вывести сообщение и завершить программу.
    – Harry
    Commented 14 окт. 2016 в 8:18

Ваш ответ

Нажимая «Отправить ответ», вы соглашаетесь с условиями пользования и подтверждаете, что прочитали политику конфиденциальности.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.