English 中文(简体)
要点和参考问题
原标题:pointers and references question
  • 时间:2009-10-09 13:30:20
  •  标签:
#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 << "
";

    b->Z(new Teste);

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

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

I would like to know if there is a memory leak in this code it works ok, the *a is set twice and the AnyNum prints different numbers on each cout << but I wonder what happened to the _Z after the setter(new Teste), I don t have much knowledge in pointers/references yet, but for the logic I guess it is being swapped for the new variable if it is leaking, is there anyway to accomplish this without having to set a to _Z again? because the address didn t change, just the direct memory allocated I was going to use *& instead of just pointers, but would it make difference?

最佳回答

此线有传闻:

b->Z(new Teste);

由于职能的定义:

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

它像Z一样,没有争论,就应该成为骗局,有争论。 我怀疑你打算这样做:

void Z(Teste *value){
    value->AnyNum = 100;
    _Z = 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 << "
";

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

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

    return 0;
}
问题回答

是:

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

编造的指派操作员不会提供深厚的复印件,而是将提供一份浅印本。 你们必须做的是,为<代码>测试<<<>/代码>写出合适的指派操作员(以及可能的话一份复印件)。 另外,在删除前,如果一名协调人是全国人民代表大会,你不必检查:

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

You ve got 另一个问题:_Z不是你应当使用的识别资料。 总的说来,最好避免主要强调,特别是重复强调或强调后面的资本信函保留执行。

What a mess! The whole program is very hard to read because of the choice of identifier names to start with:

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

I find that very ugly. When using classes it seems very un-nessacery. You could use it where a variables is going out of scope but it is a waster of time in the destructor. I think it would be asier to wrap the code in some smart pointer:


class Teste
{
    private:
        Teste *_Z;

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

Ok. You have a pointer member that you delete in the destructor. This means you are taking ownership of the pointer. This means that the ule of four applies (similar to the rule of three but applicable to ownership rules). This means you basically need to write 4 methods or the compiler generated versions will mess up your code. The methods you should write are:

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

Your code only has two of these. You need to write the other two. Or your object should not take ownership of the RAW pointer. ie. use a Smart Pointer.


Teste *_Z;

This is not allowed. Identifiers beginning with an underscore and a capitol letter are reserved. You run the risk of an OS macro messing up your code. Stop using an underscore as the first character of identifiers.


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

This is not needed. Asimple delete _Z would have been fine. _Z is going out of scope because it is in the destructor so no need to set it to NULL. The delete operator handles NULL pointers just fine.

~Test()
{    delete _Z;
}

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

What happens if you call Z() multiple times (PS putting the * next to the Z rather than next to the Teste make it hard to read). Each time you call Z() the member variable _Z is given a new value. But what happens to the old value? Basically you are leaking it. Also by returning a pointer to an object owned inside Teste you are giving somebody else the opportunity to abuse the object (delete it etc). This is not good. There is no clear ownership indicated by this method.

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

You are copying the content of a newly constructed object (that contains a pointer) into another dynamically created object! What happens if _Z had not been allocated first. The constructor sets it to NULL so there is no guarantee that it has a valid value. Any object you allocate you should also delete. But here value is dynamically allocated passed into Z but never freed. The reason you get away with this is because the pointer is c opied into _Z and _Z is deleted when its destructor is destroyed.


Teste *b = new Teste, *a;

That s really heard to read. Don;t be lazy write it out properly. This is considered bad style and you would never get past any code review with that.

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

a = b->Z();

a. a。 但谁正在摧毁物体a或b?

b->Z(new Teste);

之后,它就变得过于复杂。

(我试图补充这一评论,但修改了该法典。)

我强烈建议不要使用

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

(实际上不是答案,而是评论没有做)

你界定你的宏观方法容易出现微妙的错误(而迄今为止没有人发现这一点就证明了这一点)。 考虑你的法典:

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;

It s not a big deal, given that particular if condition, but it will blow-up with anything else and you will spend ages trying to figure out why your pointers are suddenly NULL. That s why inline functions are preferred to macros - it s more difficult to mess them up.


Edit: ok, you used comma in your macro definition so you are safe... but I would still say it s safer to use [inline] function in this case. I m not one of the do-not-use-macros-ever guys, but I wouldn t use them unless they are strictly necessary and they are not in this case

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

以及

b->Z(new Teste);

造成记忆泄露

新的测试从未删除,而你正在把新物体作为参数分配,然后用_Z = *价值复制其中的任何物体,但在呼吁后不会删除。

如果是,请写

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

更好

of course most would use boost::shared_ptr or something similar to avoid caring about delete 以及 such





相关问题
热门标签