указатели и ссылки на вопрос

Question

#ifndef DELETE
    #define DELETE(var) delete var, var = NULL
#endif

using namespace std;

class Teste {
    private:
        Teste *_Z;

    public:
    Teste(){
        AnyNum = 5;
        _Z = NULL;
    }
    ~Teste(){
        if (_Z != NULL)
            DELETE(_Z);
    }

    Teste *Z(){
        _Z = new Teste;
        return _Z;
    }
    void Z(Teste *value){
        value->AnyNum = 100;
        *_Z = *value;
    }

    int AnyNum;
};

int main(int argc, char *argv[]){
    Teste *b = new Teste, *a;

    a = b->Z();

    cout << "a->AnyNum: " << a->AnyNum << "\n";

    b->Z(new Teste);

    cout << "a->AnyNum: " << a->AnyNum << "\n";

    //wdDELETE(a);
    DELETE(b);
    return 0;
}

Я хотел бы знать, есть ли утечка памяти в этом коде все работает нормально, *a устанавливается дважды, а AnyNum печатает разные цифры на каждом cout << но мне интересно, что случилось с _Z после установщика (new Teste), я пока не очень разбираюсь в указателях / ссылках, но, по логике, я думаю, что он заменяется новой переменной если он протекает, есть ли способ выполнить это, не устанавливая снова значение _Z? потому что адрес не изменился, просто выделена прямая память Я собирался использовать *& вместо просто указателей, но будет ли это иметь значение?

Answer 1

В этой строке есть утечка памяти:

b->Z(new Teste);

из-за определения функции:

void Z(Teste *value){
    value->AnyNum = 100;
    *_Z = *value;
}

Похоже, что Z без аргументов должен был быть получателем, а с аргументами - установщиком. Я подозреваю, что вы хотели сделать:

void Z(Teste *value){
    value->AnyNum = 100;
    _Z = value;
}

(обратите внимание на третью строку) То есть, присвойте указатель «value» указателю «_Z» вместо того, чтобы копировать то значение, на которое указывает указатель Z. При этом первая утечка памяти будет устранена, но код все равно будет иметь ее, поскольку _Z мог содержать указатель. Так что вам нужно сделать:

void Z(Teste *value){
    value->AnyNum = 100;
    delete _Z; // you don't have to check for null
    _Z = value;
}

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

using namespace std;

class Teste {
    private:
        boost::shared_ptr<Teste> Z_;

    public:
    Teste() : AnyNum(5), Z_(NULL)
    { }

    boost::shared_ptr<Teste> Z()
    {
        Z_.reset(new Teste);
        return Z_;
    }

    void Z(boost::shared_ptr<Teste> value)
    {
        value->AnyNum = 100;
        Z_ = value;
    }

    int AnyNum;
};

int main(int argc, char *argv[]){
    boost::shared_ptr<Teste> b = new Teste, a;

    a = b->Z();

    cout << "a->AnyNum: " << a->AnyNum << "\n";

    b->Z(boost::shared_ptr<Teste>(new Teste));

    cout << "a->AnyNum: " << a->AnyNum << "\n";

    return 0;
}

Answer 2

Да, есть:

void Z(Teste *value)
{
   value->AnyNum = 100;
   *_Z = *value; // you need assignment operator
}

Оператор присваивания, сгенерированный компилятором, не будет делать глубокую копию, вместо этого он будет делать поверхностную копию. Вам нужно написать подходящий оператор присваивания (и, возможно, конструктор копирования) для Teste. Кроме того, вам не нужно проверять, имеет ли указатель значение NULL, прежде чем удалять его:

~Teste()
{
   // no need for checking. Nothing will happen if you delete a NULL pointer
   if (_Z != NULL)
     DELETE(_Z);
}

Answer 3

Какой беспорядок! Всю программу очень трудно читать из-за выбора имен идентификаторов для начала:

#ifndef DELETE
    #define DELETE(var) delete var, var = NULL
#endif

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

---

class Teste
{
    private:
        Teste *_Z;

    public:
        Teste()
        ~Teste()    // Delete the _Z pointer.
        Teste *Z();
        void Z(Teste *value);
};

Ok. У вас есть указатель, который вы удаляете в деструкторе. Это означает, что вы вступаете во владение указателем. Это означает, что применяется правило четырех (аналогично правилу трех, но применимо к правилам владения). Это означает, что вам нужно написать 4 метода, иначе сгенерированные компилятором версии испортят ваш код. Методы, которые вы должны написать:

A Normal (or default constructor)
A Copy constructor
An Assignment operator
A destructor.

Ваш код имеет только два из них. Вам нужно написать два других. Или ваш объект не должен вступать во владение указателем RAW. то есть. используйте Smart Pointer.

---

Teste *_Z;

Это не разрешено. Идентификаторы, начинающиеся с подчеркивания и заглавной буквы, зарезервированы. Вы рискуете, что макрос ОС испортит ваш код. Прекратите использовать подчеркивание в качестве первого символа идентификаторов.

---

~Teste(){
    if (_Z != NULL)
            DELETE(_Z);
}

Это не нужно. Простое удаление _Z было бы хорошо. _Z выходит из области видимости, потому что находится в деструкторе, поэтому нет необходимости устанавливать его в NULL. Оператор удаления отлично обрабатывает указатели NULL.

~Test()
{    delete _Z;
}

---

Teste *Z(){
    _Z = new Teste;
    return _Z;
}

Что произойдет, если вы вызовете Z () несколько раз (PS, ставя * рядом с Z, а не рядом с Тестом, затрудняет чтение). Каждый раз, когда вы вызываете Z (), переменной-члену _Z присваивается новое значение. Но что происходит со старым значением? По сути, вы это пропускаете. Также, возвращая указатель на объект, принадлежащий внутри Teste вы даете кому-то возможность злоупотреблять объектом (удалить его и т. д.). Это не хорошо. Этот метод не указывает на явное право собственности.

Teste& Z()
{
    delete _Z;       // Destroy the old value
    _Z = new Teste;  // Allocate a new value.
    return *_Z;      // Return a reference. This indicates you are retaining ownership.
                     // Thus any user is not allowed to delete it.
                     // Also you should note in the docs that it is only valid
                     // until the next not const call on the object
}

---

void Z(Teste *value){
    value->AnyNum = 100;
    *_Z = *value;
}

Вы копируете содержимое вновь созданного объекта (который содержит указатель) в другой динамически созданный объект! Что произойдет, если _Z не был выделен первым. Конструктор устанавливает его в NULL, поэтому нет гарантии, что он имеет допустимое значение. Любой объект, который вы размещаете, вы также должны удалить. Но здесь значение динамически распределяется, передается в Z, но никогда не освобождается. Причина, по которой вам это сойдет с рук, заключается в том, что указатель установлен в _Z и _Z удаляется при уничтожении его деструктора.

---

Teste *b = new Teste, *a;

Это действительно слышно читать. Не ленись, напиши это правильно. Это считается плохим стилем, и вы никогда не пройдете ни один обзор кода с этим.

Teste* b = new Teste;
Teste* a; // Why not set it to NULL

---

a = b->Z();

Получение объекта ab для a. Но кто уничтожал объект а или б?

b->Z(new Teste);

После этого он становится слишком запутанным.

Answer 4

У вас есть другая проблема: _Z - это не идентификатор, который вы должны использовать. В общем, лучше избегать начальных подчеркиваний, в частности, для реализации зарезервированы двойные подчеркивания или подчеркивания, за которыми следует заглавная буква.

Answer 5

(на самом деле не ответ, но комментарий не подходит)

То, как вы определили свой макрос, подвержено незначительным ошибкам (и тот факт, что никто до сих пор его не заметил, просто доказывает это). Рассмотрим ваш код:

if (_Z != NULL) // yes, this check is not needed, but that's not the point I'm trying to make
                DELETE(_Z);

Что происходит после прохода препроцессора:

if (_Z != 0)
        delete _Z; _Z = 0;

Если у вас все еще проблемы с просмотром, позвольте мне сделать правильный отступ:

if (_Z != 0)
        delete _Z;
_Z = 0;

Это не имеет большого значения, учитывая это конкретное условие if, но оно взорвется чем-то еще, и вы потратите возрастов , пытаясь выяснить, почему ваши указатели вдруг становятся НУЛЕВЫМИ. Вот почему встроенные функции предпочтительнее макросов - их сложнее испортить. <Ч /> Редактировать: хорошо, вы использовали запятую в своем определении макроса, чтобы вы были в безопасности ... но я все равно сказал бы, что в этом случае безопаснее использовать функцию [inline]. Я не один из тех, кто никогда не использует макросы, но я бы их не использовал, если бы они не были строго необходимы, и в этом случае они не нужны

Answer 6

(Я пытался добавить это как комментарий, но это испортило код ..)

Я бы также настоятельно рекомендовал не использовать

#ifndef DELETE
  #define DELETE(var) delete var, var = NULL
#endif

но вместо этого что-то вроде

struct Deleter
{
  template< class tType >
  void operator() ( tType*& p )
  {
    delete p;
    p = 0;
  }
};

использование:

Deleter()( somePointerToDeleteAndSetToZero );

Answer 7

void Z (Teste * value) { значение-> AnyNum = 100; * _Z = * значение; }

и

b->Z(new Teste);

создает утечку памяти

«Новый Тест» никогда не удаляется, вместо этого вы выделяете новый объект в качестве параметра, затем копируете все, что есть, используя значение * _Z = *, но объект не удаляется после вызова.

если бы вы написали

Test* param - new Teste;
b->Z(param)
delete param;

было бы лучше

конечно, большинство будет использовать boost :: shared_ptr или что-то подобное, чтобы избежать заботы об удалении и тому подобное