2010-09-18 19 views
8

Próbuję nauczyć się "wielkiej trójki" w C++. Udało mi się zrobić bardzo prosty program dla "wielkiej trójki" .. ale nie jestem pewien jak używać wskaźnika obiektu. Oto moja pierwsza próba.C++ Konstruktor kopiowania + obiekt wskaźnika

mam wątpliwości, kiedy pisałem ten ...

Pytania

  1. Czy to jest prawidłowy sposób wdrożyć konstruktora domyślnego? Nie jestem pewien, czy muszę to mieć, czy nie. Ale to, co znalazłem w innym wątku na temat konstruktora kopiowania ze wskaźnikiem, polega na tym, że muszę przydzielić miejsce dla tego wskaźnika przed skopiowaniem adresu w konstruktorze kopiującym.
  2. Jak przypisać zmienną wskaźnika do konstruktora kopiowania? Sposób, w jaki napisałem w Copy Constructor może być błędny.
  3. Czy muszę zaimplementować ten sam kod (z wyjątkiem powrotu) zarówno dla konstruktora kopiowania, jak i operatior =?
  4. Czy mam rację mówiąc, że muszę usunąć wskaźnik w destruktorze?

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

Realizacja

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

Z góry dzięki.

+2

To niepotrzebne przypisanie łańcucha pustego na '' data' w swoim konstruktorze TreeNode', ponieważ domyślny konstruktor 'std :: string' zrobi to samo. –

+3

Konstruktor kopii i operator przypisania mają nieco różniące się semantyki kopiowania. To jest mylące dla użytkowników twojej klasy. – sbi

+0

hi sbi, Masz na myśli, powinienem użyć dokładnej kopii tego samego kodu zarówno dla konstruktora kopii, jak i operatora? –

Odpowiedz

21

Czy mam rację mówiąc, że muszę usunąć wskaźnik w destruktorze?

Podczas projektowania takiego obiektu należy najpierw odpowiedzieć na pytanie: Czy obiekt jest właścicielem pamięci wskazanej przez ten wskaźnik? Jeśli tak, to oczywiście destruktor obiektu musi wyczyścić pamięć, więc tak, musi wywołać delete. I wydaje się, że to twój zamiar z podanym kodem.

Jednak w niektórych przypadkach możesz chcieć mieć wskaźniki odnoszące się do innych obiektów, których żywotność ma być zarządzana przez coś innego. W takim przypadku nie chcesz wywoływać delete, ponieważ jest to obowiązkiem innej części programu, aby to zrobić. Co więcej, zmienia to cały następny projekt, który trafia do konstruktora kopiowania i operatora przypisania.

Będę kontynuować udzielanie odpowiedzi na pozostałe pytania, zakładając, że chcesz, aby każdy obiekt TreeNode posiadał prawa własności do lewego i prawego obiektu.

Czy to właściwy sposób na zastosowanie domyślnego konstruktora?

Nie trzeba zainicjować left i right wskaźniki NULL (lub 0, jeśli wolisz). Jest to konieczne, ponieważ niezainicjowane wskaźniki mogą mieć dowolną wartość. Jeśli twój kod zawsze domyślnie konstruuje TreeNode, a następnie niszczy go, nigdy nie przypisując niczego do tych wskaźników, wówczas usuwane będzie wywołanie niezależnie od wartości początkowej. Więc w tym projekcie, jeśli te wskaźniki nie wskazują na nic, musisz zagwarantować, że są ustawione na NULL.

Jak przypisać zmienną wskaźnika do konstruktora kopiowania? Sposób, w jaki napisałem w Copy Constructor może być błędny.

Linia left = new TreeNode(); tworzy nowy obiekt TreeNode i ustawia left, aby wskazywało na niego. Linia left = node.left; ponownie przypisuje wskaźnik do wskazania dowolnego obiektu TreeNode o numerze node.left. Są z tym dwa problemy.

Problem 1: Nic teraz nie wskazuje na ten nowy TreeNode. Jest zagubiony i staje się przeciekiem pamięci, ponieważ nic nie może go zniszczyć.

Problem 2: Teraz zarówno left, jak i node.left wskazują na ten sam obiekt TreeNode. Oznacza to, że obiekt, który jest kopiowany, a obiekt, z którego pobiera wartości, będzie uważał, że jest właścicielem tego samego TreeNode i oba wywołają usuwanie na nim w swoich destruktorach. Wywołanie dwukrotnego usunięcia tego samego obiektu jest zawsze błędem i spowoduje problemy (w tym ewentualne awarie lub uszkodzenie pamięci).

Ponieważ każdy TreeNode ma swoje lewe i prawe węzły, prawdopodobnie najbardziej sensownym rozwiązaniem będzie wykonanie kopii. Więc można napisać coś podobnego do:

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

Czy muszę wdrożyć ten sam kod (z wyjątkiem zwrotu) zarówno kopię konstruktora i operatior =?

Prawie na pewno. A przynajmniej kod każdego z nich powinien mieć taki sam efekt końcowy. Byłoby bardzo mylące, gdyby kopiowanie konstrukcji i przydział miały różne efekty.

EDYCJA - Powyższy akapit powinien być: Kod w każdym powinien mieć taki sam efekt końcowy, ponieważ dane są kopiowane z innego obiektu. Będzie to często obejmować bardzo podobny kod. Jednak operator przypisania prawdopodobnie musi sprawdzić, czy coś zostało już przypisane do left i right i tak wyczyścić je. W konsekwencji może również potrzebować uważać na własny przydział lub napisać w sposób, który nie spowoduje niczego złego w trakcie samozasadzenia.

W rzeczywistości istnieją sposoby implementacji jednego przy użyciu drugiego, tak aby rzeczywisty kod, który manipuluje zmiennymi składowymi, jest zapisywany tylko w jednym miejscu. Inne pytania na temat SO omówiły to, na przykład: this one.

+1

W jaki sposób byłeś jedynym, który zwrócił uwagę na "nowy wyciek pamięci TreeNode(); –

+1

+1 za zrozumienie problemu i podanie dobrego rozwiązania. Ta odpowiedź jest zdecydowanie lepsza niż moja. :) –

+0

Dziękuję bardzo za bardzo szczegółową odpowiedź .. –

1

Czy to właściwy sposób na zastosowanie domyślnego konstruktora?

Nie, nazywając delete na coś, co nie zostało przydzielone za pomocą new wywołuje zachowanie niezdefiniowane (w większości przypadków wynika to do katastrofy aplikacji)

Ustaw wskaźniki do NULL w domyślnej konstruktora.

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

Nie widzę takich problemów z resztą kodu. Twój operator przydziału płytko kopiuje węzły, podczas gdy twój konstruktor kopiowania głęboko je kopiuje.

Stosuj odpowiednie podejście zgodnie z wymaganiami. :-)

EDIT:

Zamiast przypisywania wskaźniki w domyślnego konstruktora używać initialization list

+0

Dzięki. A co z innymi pytaniami? –

+0

Zobacz zmiany, starałem się krótko odpowiedzieć na twoje pytania. :-) –

+0

Dzięki .. Czy muszę wstawić "left = new TreeNode();" w konstruktorze kopii? Czy to konieczne? W przeciwnym razie usunę go ... –

8

Nawet lepiej, myślę

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

Plus trzeba usunąć wskaźniki " "w lewo" i "w prawo" w operacji przypisania, lub będziesz miał wyciek pamięci

+4

+1 ode mnie za wzmiankę o liście inicjalizacyjnej. –

1

Czy mogę też zaproponować boost :: shared_ptr z biblioteki boost (jeśli można go użyć) zamiast prostych wskaźników? To rozwiąże wiele problemów, które możesz mieć z nieprawidłowymi wskazówkami, głębokimi kopiami e.t.c.

+0

Rzeczywiście, napotkasz problemy z kodem, jak to jest, gdy przypisujesz jeden węzeł drzewa z drugiego, a następnie oba są usuwane: dzieci zostaną usunięte dwa razy, ale zostały przydzielone tylko raz. Mogę więc tylko wspierać tę sugestię, aby używać inteligentnych wskaźników. –

3

To w jaki sposób to zrobić:
Bo zarządzają dwa zasoby w tym samym obiekcie robi to poprawnie staje się nieco bardziej skomplikowane (dlatego polecam nigdy zarządzający więcej niż jednego zasobu w obiekcie) . Jeśli używasz idiomu kopiowania/wymiany, złożoność jest ograniczona do konstruktora kopii (co nie jest banalne, aby uzyskać poprawną gwarancję wyjątku ).

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

Najtrudniejsze teraz:

/* 
* 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(); 
} 
+0

+1 Nawet jeśli może to być nieco zaawansowane dla kogoś, kto dopiero uczy się podstaw zarządzania wskaźnikami i wdrażania dużych trzech operatorów, jest to nadal poprawne rozwiązanie "o dużej sile przemysłowej". – TheUndeadFish

+0

@TheUndeadFish: Jest złożony, ponieważ do zarządzania obiektem są dwa zasoby. Jeśli byłby to pojedynczy zasób, wymagałoby to zasadniczo zerowego kodu (dlatego obiekty IMHO powinny zarządzać tylko jednym zasobem na raz). –

+0

Wielkie dzięki, Martin .. Spróbuję skompilować to na mój koniec .. Używam MinGW .... Nie użyłem auto_ptr i release() przed .. Myślę, że powód, dlaczego potrzebuję dwa obiekty jest to, że muszę mieć lewy węzeł i prawy węzeł .... –

Powiązane problemy