Почему этот деструктор вызывается сразу после создания? - PullRequest
6 голосов
/ 15 января 2012

У меня есть следующий класс:

class FixedByteStream {
public:
FixedByteStream() : size(0), address(NULL), existing(false) {}
FixedByteStream(int length) : existing(false) {
    size = length;
    address = new char[length];
}
FixedByteStream(int length, char* addr) : existing(true) {
    size = length;
    address = addr;
}
FixedByteStream(string* str, bool includeNull = false) : existing(true) {
    size = (*str).length();
    address = const_cast<char*>((*str).c_str());
    if (includeNull){
        ++size;
    }
}
~FixedByteStream() {
    if (existing == false) {
        delete [] address;
    }
    address = NULL;
}
int getLength() {
    return size;
}
char* getAddressAt(int index) {
    return &(address[index]);
}


char& operator[] (const int index) {
    return address[index];
}
operator char*() {
    return address;
}

private:
    bool existing;
    int size;
    char* address;
};

И очень простой тест, способный вызвать проблему:

FixedByteStream received;
received = FixedByteStream(12);
received[0] = 't';

Valgrind предупреждает о недопустимой записи, и отладка имеетпоказано почему.FixedByteStream received; вызывает конструктор без аргументов (что довольно глупо, потому что он не может делать что-либо).received = FixedByteStream(12); вызывает конструктор с целочисленным аргументом ... и затем немедленно вызывает деструктор для самого себя , делая объект недействительным.По какой-то причине это все еще работает, но я бы предпочел, чтобы его не помещали в такое странное положение, в котором бы появлялись предупреждения.

Итак, почему он там вызывается?Я мог бы кое-что понять, если бы деструктор назывался first , чтобы избавиться от бесполезного временного объекта (не то, что ему нужно), но я использовал этот тип шаблона объявления-теперь-назначения-позже практически вездеи никогда не было такой проблемы раньше.

Ответы [ 6 ]

9 голосов
/ 15 января 2012

Вам не хватает оператора присваивания. Запомните правило трех (или пяти).

Проблема примерно такая:

T t; // default constructed t
t = T(2); // T(2) constructor with a single argument, assignment operator= called with this == &t

Вы не предоставляете оператор присваивания, поэтому значение указателя во временном хранилище просто копируется в t, а затем указанная память удаляется в деструкторе временного.

Также: не иметь конструктора по умолчанию, если созданный объект недопустим.

6 голосов
/ 15 января 2012

Если у вашего объекта есть какой-либо определяемый пользователем конструктор, он всегда создается с использованием конструктора.Простое определение объекта без аргументов конструктора использует конструктор по умолчанию независимо от того, перезаписывается ли объект впоследствии.То есть

FixedByteStream received;

вызовет конструктор по умолчанию.Следующая строка немного интереснее:

received = FixedByteStream(12);

Эта строка создает временный FixedByteStream с аргументом 12.Внутренне это выделит некоторую память, но поскольку временные значения уничтожаются в конце полного выражения (в данном случае, по существу, при достижении точки с запятой), это не принесет вам большой пользы.Как только этот временный объект создан, он присваивается received с использованием автоматически сгенерированного назначения копирования, которое выглядело бы примерно так, если бы вы написали его вручную:

FixedByteStream& FixedByteStream::operator= (FixedByteStream& other) {
    this->existing = other.existing;
    this->size     = other.size;
    this->address  = other.address;
    return *this;
}

То есть после выполнения этого назначенияВы должны идентичные копии FixedByteStream, одна из которых собирается быть уничтоженной и высвободит только что выделенные ресурсы.Это явно не то, что вам нужно, т.е. вам определенно нужно реализовать оператор присваивания копирования, чтобы ваш класс хорошо себя вел.В общем, наличие деструктора, который делает что-нибудь интересное, является хорошим намеком на то, что вам также понадобится оператор присваивания.Фактически, существует еще одна из сгенерированных операций, а именно конструктор копирования, который выполняет примерно то же, что и назначение копирования, за исключением того, что копирование создает элементы вместо их назначения.Это также не то, что вы хотите от своего класса.

Теперь возникает интересный вопрос: как можно исправить FixedByteStream?По сути, вам нужно либо использовать подсчет ссылок, чтобы отслеживать, сколько объектов в данный момент просматривает FixedByteStream, выделить копию содержимого, либо вам нужно использовать семантическую поддержку перемещения (или ссылки на значения), которая является толькодоступно в C ++ 2011.Если вы действительно не знаете, что делаете, я рекомендую копировать поток во всех случаях и оставлю более продвинутый подход на будущее.

4 голосов
/ 15 января 2012

Шаг за шагом:

//create a new object using the default constructor
//I don't see why you think it's stupid that the constructor is called
//it's doing what you're telling it to do
FixedByteStream received;

//FixedByteStream(12) creates a temporary object
//you then copy this object in the received object you already have
//you're using the default operator =
//the temporary object is deleted after it is copied to received
received = FixedByteStream(12);

//received is a valid object
received[0] = 't';

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

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

Вы в основном инициализируете некоторый объект в стеке.

Я упросту ваш случай:

class A
{
    A() {}
    A(const A& other) {}
    A& operator = (const A& other) {}
};

Давайте поговорим о сфере:

{ //begin scope
  A a;  //object a is created here
        //default constructor is called
} //a is destroyed here
  //destructor is called

Пока все хорошо.

Теперь назначение:

{
   //two objects are created with default constructor
   A a;
   A b;
   //object b is assigned to a
   //operator = will be called
   //both objects are still alive here
   a = b;
   //...
} // a and b will be destroyed, destructor called

На последней части:

{
   A a;
   a = A();
}

в значительной степени эквивалентно:

{
   A a;
   {
      A b;
      a = b;
   }
}

Когда вы звоните a = A(), A()создает временный объект, который назначается на a, а затем уничтожается.

Таким образом, объект b в моем упрощении - это временный объект, который уничтожается.Не a, ваш оригинал, поэтому a все еще действителен.

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

1 голос
/ 15 января 2012

FixedByteStream(12) присваивается received через оператор по умолчанию =.Обратите внимание, что вы не распределили FixedByteStream(12) в куче с помощью new, а скорее разместили его в локальной области, не указав имя переменной, которая будет его содержать.

Ваш код чем-то похож на:

FixedByteStream received;
FixedByteStream temp(12);
received = temp;
received[0] = 't';

Только в моем примере temp имеет имя, а его область действия - вся функция, а в вашем тестовом коде temp hasn 'имя не существует, оно существует только для одной строки и затем уничтожается.

Созданный вами объект FixedByteStream(12) нельзя использовать после этой строки, поскольку он даже не назван как varaible.

0 голосов
/ 15 января 2012

Объект уже инициализирован в этой строке

FixedByteStream received;

В сети

received = FixedByteStream(12);

Вы реинициализируете это. Правильный способ сделать это:

FixedByteStream received(12);
// Or
FixedByteStream *received;
received = new FixedByteStream(12);

(я бы точно выбрал первый)

0 голосов
/ 15 января 2012

Кажется, вы не понимаете жизненный цикл объекта и по ошибке интерпретируете этот код как код Java.

Когда вы пишете FixedByteStream received;, объект типа FixedByteStream создается с помощью конструктора без аргументов.И когда вы пишете received = FixedByteStream(12);, создается другой объект, вызывается оператор =, и вновь созданный объект удаляется.

Также вы не переопределяли оператор =, поэтому объект копируется байтами, что неправильно.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...