Должен ли я проверить NULL в моем операторе присваивания? - PullRequest
Новая
       12

Должен ли я проверить NULL в моем операторе присваивания?

1 голос
/ 08 октября 2011

У меня есть класс 

class Node {

  public:
    int value;
    Node * next;

    Node();
    Node(const Node& other);
    Node& operator= (const Node& other);
};

Это не очень полезно, но у него есть переопределенный оператор присваивания.Все там есть public, потому что я такой открытый и дружелюбный парень.

Теперь в других местах у меня есть массив этих узлов: 

Node * nodes = new Node[15];

Когда я пытаюсь назначить узел длямой массив узлов: 

nodes[0] = Node();

Я получаю огромный ужасный сбой.

Мой оператор присваивания выглядит так: 

Node& Node::operator= (const Node& other) {

  // watch out for self assignment
  if (this == &other) return *this;

  delete this->next;
  this->next = new Node(*(other.next)); // call the copy constructor
  this->value = other.value;

  return *this;
}

У меня такое чувство, что я долженпроверять, является ли this NULL, прежде чем пытаться разыменовать его членов.Есть мысли о том, что может быть не так?

Ответы [ 4 ]

2 голосов
/ 08 октября 2011

Вы никогда не должны проверять, что this равно NULL;Запрещается вызывать нестатическую функцию-член, отличную от допустимого объекта.

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

1 голос
/ 08 октября 2011

Проблема в том, что вы разыменовываете other.next, что может быть NULL. Поэтому перед разыменованием вы должны проверить, является ли other.next нулевым: 

this->next = other.next ? new Node(*other.next) : 0;

Разыменование нулевого указателя не определено, поэтому, если вы сделаете это, может произойти все что угодно - он НЕ может сразу аварийно завершить работу и вместо этого улететь в неожиданные места, что может сбить вас с толку и отладчика.

0 голосов
/ 08 октября 2011

Я удваиваю, чтобы вы не проверяли это на NULL.Я полагаю, что ваше право собственности не совсем понятно.Кому принадлежит следующий узел?

Этот -> следующий или другой -> следующий может быть нулевым, поэтому проверьте перед удалением / разыменованием.

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

0 голосов
/ 08 октября 2011

this никогда не будет NULL, однако в вашем примере this->next будет либо NULL, либо недопустимой ссылкой для nodes[0], поскольку этот объект никогда не был инициализирован (я не совсем уверен что делает компилятор, возможно, он заполняет массив нулями, а может и нет). Я предлагаю вам принять меры против delete this->next; для указателей NULL и убедиться, что ваш массив очищен после выделения. То есть внести эти изменения:

Node * nodes = new Node[15] {0};

И

if (this->next) delete this->next;

...