C ++ Copy Constructor + Pointer Object - PullRequest
       32

C ++ Copy Constructor + Pointer Object

8 голосов
/ 18 сентября 2010

Я пытаюсь выучить "большую тройку" на C ++. Мне удалось создать очень простую программу для "большой тройки" ... но я не уверен, как использовать указатель объекта. попытка.

У меня есть сомнения, когда я писал это ...

Вопросы

  1. Это правильный способ реализации конструктора по умолчанию? Я не уверен, нужно ли мне это или нет. Но что я нашел в другом потоке о конструкторе копирования с указателем, так это то, что мне нужно выделить место для этого указателя перед копированием адреса в конструкторе копирования ..
  2. Как назначить указатель переменной в конструкторе копирования? Способ, который я написал в конструкторе копирования, может быть неправильным.
  3. Нужно ли реализовывать один и тот же код (кроме возврата) как для конструктора копирования, так и для оператора =?
  4. Правильно ли я сказал, что мне нужно удалить указатель в деструкторе?

    class TreeNode
    {
    public:  
       TreeNode(); 
       TreeNode(const TreeNode& node);
       TreeNode& operator= (const TreeNode& node);
       ~TreeNode();
    private:
       string data;
       TreeNode* left;
       TreeNode* right;
       friend class MyAnotherClass;
    };
    

Осуществление

TreeNode::TreeNode(){

    data = "";  

}

TreeNode::TreeNode(const TreeNode& node){
     data = node.data;

     left = new TreeNode();
     right = new TreeNode();

     left = node.left; 
     right = node.right;
}

TreeNode& TreeNode::operator= (const TreeNode& node){
     data = node.data;
     left = node.left;
     right = node.right;
     return *this;
}

TreeNode::~TreeNode(){
     delete left;
     delete right;
}

Заранее спасибо.

Ответы [ 5 ]

22 голосов
/ 18 сентября 2010

Правильно ли я сказал, что мне нужно удалить указатель в деструкторе?

Всякий раз, когда проектируете такой объект, вам сначала нужно ответить на вопрос: владеет ли объект памятью, на которую указывает этот указатель?Если да, то, очевидно, деструктор объекта должен очистить эту память, так что да, он должен вызвать delete.И это, похоже, ваше намерение в отношении данного кода.

Однако в некоторых случаях вы можете захотеть иметь указатели, которые ссылаются на другие объекты, чья жизнь должна управляться чем-то другим.В этом случае вы не хотите вызывать delete, потому что это является обязанностью какой-то другой части программы.Кроме того, это изменяет весь последующий дизайн, который входит в конструктор копирования и оператор присваивания.

Я продолжу отвечать на остальные вопросы в предположении, что вы хотите, чтобы каждый объект TreeNode имел право собственности налевый и правый объекты.

Это правильный способ реализации конструктора по умолчанию?

Нет.Вам нужно инициализировать указатели left и right в NULL (или 0, если вы предпочитаете).Это необходимо, потому что неинициализированные указатели могут иметь любое произвольное значение.Если ваш код по умолчанию создает TreeNode, а затем уничтожает его, не назначая ничего указателям, тогда будет вызываться delete для любого начального значения.Так что в этом проекте, если эти указатели ни на что не указывают, вы должны гарантировать, что они установлены в NULL.

Как назначить переменную указателя в конструкторе копирования?Способ, который я написал в конструкторе копирования, может быть неправильным.

Строка left = new TreeNode(); создает новый объект TreeNode и устанавливает left для указания на него.Строка left = node.left; переназначает этот указатель так, чтобы он указывал на любой объект TreeNode, на который указывает node.left.Есть две проблемы с этим.

Проблема 1: Ничто теперь не указывает на этот новый TreeNode.Он теряется и становится утечкой памяти, потому что ничто не может его уничтожить.

Проблема 2: Теперь и left, и node.left в конечном итоге указывают на один и тот же TreeNode.Это означает, что объект, создаваемый при копировании, и объект, от которого он получает значения, будут думать, что они владеют тем же TreeNode, и оба будут вызывать delete в своих деструкторах.Вызов delete дважды для одного и того же объекта всегда является ошибкой и вызовет проблемы (в том числе, возможно, сбои или повреждение памяти).

Поскольку каждый TreeNode имеет свой левый и правый узлы, то, вероятно, наиболее разумным является сделатькопии.Поэтому вы должны написать что-то похожее на:

TreeNode::TreeNode(const TreeNode& node)
    : left(NULL), right(NULL)
{
    data = node.data;

    if(node.left)
        left = new TreeNode(*node.left);
    if(node.right)
        right = new TreeNode(*node.right);
}

Нужно ли реализовывать один и тот же код (кроме возврата) как для конструктора копирования, так и для оператора =?

Почти наверняка.Или, по крайней мере, код в каждом должен иметь одинаковый конечный результат.Было бы очень странно, если бы структура копирования и назначение имели разные эффекты.

РЕДАКТИРОВАТЬ - вышеупомянутый параграф должен быть: Код в каждом должен иметь тот же конечный результат, что данные копируются из другогообъект.Это часто будет включать очень похожий код.Однако оператору присваивания, вероятно, необходимо проверить, было ли что-либо уже присвоено left и right, и очистить их.Как следствие, может также потребоваться следить за самопредставлением или написать его так, чтобы во время самоназначения не происходило ничего плохого.

На самом деле, есть способы реализоватьодин использует другой, так что фактический код, который манипулирует переменными-членами, написан только в одном месте.Об этом уже говорили другие вопросы по SO, такие как this .

8 голосов
/ 18 сентября 2010

Еще лучше, я думаю

 TreeNode::TreeNode():left(NULL), right(NULL)
 {
   // data is already set to "" if it is std::string
 }

плюс вы должны удалить указатели 'left' и 'right' в операции присваивания, иначе у вас будет утечка памяти

3 голосов
/ 18 сентября 2010

Вот как я бы это сделал:
Поскольку вы управляете двумя ресурсами в одном и том же объекте, правильное выполнение этого становится немного сложнее (поэтому я рекомендую никогда не управлять более чем одним ресурсом в объекте).Если вы используете идиому копирования / замены, тогда сложность ограничивается конструктором копирования (что нетривиально получить для строгой гарантии исключения ).

TreeNode::TreeNode()
    :left(NULL)
    ,right(NULL)
{}

/*
 * Use the copy and swap idium
 * Note: The parameter is by value to auto generate the copy.
 *       The copy uses the copy constructor above where the complex code is.
 *       Then the swap means that we release this tree correctly.
 */ 
TreeNode& TreeNode::operator= (const TreeNode node)
{
     std::swap(data,  node.data);
     std::swap(left,  node.left);
     std::swap(right, node.right);
     return *this;
}

TreeNode::~TreeNode()
{
     delete left;
     delete right;
}

Сложная частьсейчас:

/*
 * The copy constructor is a bit harder than normal.
 * This is because you want to provide the `Strong Exception Guarantee`
 * If something goes wrong you definitely don't want the object to be
 * in some indeterminate state.
 *
 * Simplified this a bit. Do the work that can generate an exception first.
 * Once all this has been completed we can do the work that will not throw.
 */   
TreeNode::TreeNode(const TreeNode& node)
{
    // Do throwable work.
    std::auto_ptr<TreeNode>  nL(node.left  == null ? null : new TreeNode(*node.left));
    std::auto_ptr<TreeNode>  nR(node.right == null ? null : new TreeNode(*node.right));

    // All work that can throw has been completed.
    // So now set the current node with the correct values.
    data  = node.data;
    left  = nL.release();
    right = nR.release();
}
1 голос
/ 18 сентября 2010

Могу ли я также предложить boost :: shared_ptr из библиотеки boost (если вы можете его использовать) вместо простых указателей?Это решит множество проблем, которые могут возникнуть с недействительными указателями, глубокими копиями и т. Д.

1 голос
/ 18 сентября 2010

Это правильный способ реализации конструктора по умолчанию?

Нет, вызов delete для чего-то, что не было выделено с помощью new, вызывает неопределенное поведение (в большинстве случаевэто приводит к падению вашего приложения)

Установите ваши указатели на NULL в конструкторе по умолчанию.

TreeNode::TreeNode(){
  data = "";   //not required since data being a std::string is default initialized.
  left = NULL;
  right = NULL;   
}

Я не вижу таких проблем с остальным кодом.Ваш оператор присваивания копирует узлы, тогда как ваш конструктор копирования копирует их.

Следуйте подходящему подходу согласно вашему требованию.: -)

РЕДАКТИРОВАНИЕ :

Вместо назначения указателей в конструкторе по умолчанию используйте список инициализации

...