Конструктор копии C++ с указателями

Question

Я понимаю необходимость указателей глубокой копии (в тех случаях, когда вы хотите получить полную копию объекта), моя путаница приходит со следующим (полностью составленным примером).

#include "stdafx.h" 
#include <string> 

class a 
{ 
public: 

    a::a(std::string _sz) : 
     m_sz(_sz) 
     ,m_piRandom(new int) 
    { 
     *m_piRandom = 1; 
    }; 

    ~a() 
    { 
     delete m_piRandom; 
     m_piRandom = NULL; 
    }; 

    a::a(const a &toCopy) 
    { 
     operator=(toCopy); 
    } 

    a& a::operator=(const a &toAssign) 
    { 
     if (this != &toAssign) 
     { 
      m_sz = toAssign.m_sz; 
      if (m_piRandom) 
      { 
       // Need to free this memory! 
       delete m_piRandom; 
       m_piRandom = NULL; 
      } 

      m_piRandom = new int(*toAssign.m_piRandom); 
     } 
     return *this; 
    } 

    void SetInt(int i) 
    { 
     if (!m_piRandom) 
     { 
      m_piRandom = new int; 
     } 
     *m_piRandom = i; 
    } 

private: 

    std::string m_sz; 
    int* m_piRandom; 
}; 


int _tmain(int argc, _TCHAR* argv[]) 
{ 
    a Orig = a("Original"); 
    a New = a("New"); 
    New.SetInt(9); 
    New = Orig; 

    return 0; 
} 

Теперь в моем примере я хочу проверить сценарий, где у меня есть объект с некоторой памяти, выделенной для него, в этом случае:

a New = a("New"); 
New.SetInt(9); // This new's the memory 

выделяет память, а затем, когда мы говорим: New = Orig; Я бы ожидал утечки памяти, потому что, если бы я слепо new'd m_piRandom = new int(*toAssign.m_piRandom);, я бы потерял память, о которой он ранее указывал.

Поэтому я решил поставить следующее в операторе присваивания:

if (m_piRandom) 
      { 
       // Need to free this memory! 
       delete m_piRandom; 
       m_piRandom = NULL; 
      } 

Этот сбой код, когда следующий называется (первая линия!) a Orig = a("Original"); как он вызывает конструктор копирования (который я называю оператор присваивания для меньшего дублирования) и указатель m_piRandom установлен в 0xcccccccc. Не ноль. Поэтому он пытается удалить память, которая никогда не была выделена. Я ожидаю, что он будет работать, когда он дойдет до New = Orig;, потому что он сначала удалит его, прежде чем назначать копию. Может кто-нибудь пролить свет на это, я думаю, моя самая большая проблема заключается в том, что m_piRandom не является NULL, я также попытался определить конструктор по умолчанию, для которого NULL указывает указатель по умолчанию, но это не помогло. Извиняюсь за совершенно надуманный код ..

Благодарности

Answer 1

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

Таким образом, ваш правильный конструктор копирования будет: -

a::a(const a &toCopy) : m_sz(toCopy.m_sz), m_piRandom(new int) 
{ 
    *m_piRandom = toCopy.m_piRandom; 
} 

После реализации этого можно упростить оператор присваивания:

a& a::operator=(const a &toAssign) 
{ 
    if (this != &toAssign) 
    { 
     m_sz = toAssign.m_sz; 
     if (m_piRandom)   //<<<<< No need to check this as it should always be 
     {       //<<<<< initialized by constructors. 
      delete m_piRandom;  
      m_piRandom = NULL; 
     } 

     m_piRandom = new int(*toAssign.m_piRandom); 
    } 
    return *this; 
} 

После удаления этих увольнений, ваш оператор присваивания выглядит

a& a::operator=(const a &toAssign) 
{ 
    if (this != &toAssign) 
    { 
     m_sz = toAssign.m_sz; 
     m_piRandom = new int(*toAssign.m_piRandom); 
    } 
    return *this; 
} 

Answer 2

Проблема с кодом, что ваш конструктор копирования не инициализирует ИНТ указатель membrer, но оператор присваивания принимает правильное значение для него. Поэтому просто инициализируйте указатель int на 0 в конструкторе копирования перед вызовом оператора присваивания.

Answer 3

Ошибка возникает из-за того, что конструктор копирования не инициализирует m_piRandom. Это означает, что переменная будет (скорее всего) заполнена мусором (все, что было в ячейке памяти при инициализации объекта).

Последовательность вызова заключается в следующем:

a::a() [doesn not initialize m_piRandom] -> a::operator= -> delete m_piRandom. 

Чтобы исправить:

a::a(const a &toCopy) 
: m_piRandom{ nullptr } // <---- add this 
{ 
    operator=(toCopy); 
} 

Edit: вы могли бы улучшить оператор присваивания резко используя copy&swap idiom.