Почему мой код C ++ не может удалить все узлы в моем BST? - PullRequest
1 голос
/ 11 февраля 2010

Предполагается, что он проходит через BST и удаляет каждый узел, включая корневой узел. Тем не менее, в конце я получаю сообщение «root все еще имеет левый узел». Почему не все узлы удалены?

void deleteTree()
{   
    deleteNode(root);
    if(root->right)
        cout << "root still has a right node" << endl;
    if(root->left)
        cout << "root still has a left node" << endl;
    root = 0;

}   

void deleteNode(node *p) 
{   
    if(p->left) 
    {   
        deleteNode(p->left);
        p->left = 0;
    }   
    if(p->right) 
    {   
        deleteNode(p->right);
        p->right = 0;
    }   

    cout << "Deleting node containing " << p->data << endl;
    delete p;
}   

Ответы [ 5 ]

6 голосов
/ 11 февраля 2010

Вы удаляете p в конце (root) и затем пытаетесь получить доступ к его содержимому в deleteTree(), где root больше не указывает на выделенную память. Результат будет неопределенным.

2 голосов
/ 11 февраля 2010

Вы просматриваете root->left после того, как вы уже удалили root, делая его доступным для использования в новом выделенном блоке.

2 голосов
/ 11 февраля 2010

Вы не должны разыменовывать root после удаления в deleteNode. Используйте отладчик, чтобы проверить, почему root->left не равно нулю.

2 голосов
/ 11 февраля 2010

Вы удаляете root. И тогда ваш код пытается получить доступ к памяти, где он был.

Вы попадаете в землю неопределенного поведения.

0 голосов
/ 11 февраля 2010

Я бы просто поменял само дерево, тогда было бы легче с ним справиться:

struct Node
{
  Node(data_type data): mLeft(), mRight(), mData(data) {}
  Node(const Node& rhs): mLeft(), mRight(), mData(rhs.mData)
  {
    if (rhs.mLeft.get()) mLeft.reset(new Node(*rhs.mLeft));
    if (rhs.right.get()) mRight.reset(new Node(*rhs.mRight));
  }
  Node& operator=(Node rhs)
  {
    this->swap(rhs);
    return *this;
  }
  ~Node() { }

  void swap(Node& rhs)
  {
    using std::swap;
    swap(mLeft, rhs.mLeft);
    swap(mRight, rhs.mRight);
    swap(mData, rhs.mData);
  }

  Node* left() const { return mLeft.get(); }
  void left(std::auto_ptr<Node> node) { mLeft= node; }

  Node* right() const { return mRight.get(); }
  void right(std::auto_ptr<Node> node) { mRight = node; }

  data_type& data() { return mData; }
  const data_type& data() const { return mData; }

private:
  std::auto_ptr<Node> mLeft;
  std::auto_ptr<Node> mRight;
  data_type mData;
};

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

Обратите внимание, что он был адаптирован для глубокого копирования, любой другой подход требует boost::shared_ptr или эквивалентный. И да std::auto_ptr оставляет вам дело с копированием самостоятельно, никакой магии там нет.

Этот дизайн намного чище, чем использование простого C-struct, когда каждый может манипулировать ресурсами. У вас все еще есть полный доступ к базовым данным через средство доступа ... но ОНИ заботятся, чтобы не вызывать неопределенное поведение ...

Конечно, вы все равно можете разбить его:

Node& node = ...
delete node.left(); // haha

Но если C ++ может защитить от непреднамеренных проблем, он оставляет дверь открытой для злого кода.

...