• Create Account

## about "delete" and "delete []"

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

35 replies to this topic

### #21xiajia  Members

Posted 16 January 2013 - 11:11 AM

thanks for Brother Bob,Cornstalks and rip-off.These recommendations so refreshing for me.I should solve the problem instead of ducking the issue about double delete.

you can and should provide it as a regular free function rather than a macro:

Not "macro higher efficiency than function"?

### #22Cornstalks  Members

Posted 16 January 2013 - 11:31 AM

Not "macro higher efficiency than function"?

Not really, no. Compilers these days are incredibly smart. They can inline functions if needed (especially little tiny, templated ones like that), just as if it were a macro. However, sometimes if you inline too many things, you just end up bloating your code (which can actually slow you down). By making it a function, you are letting the compiler decide if its better to inline the function or not to (most of the time, for a tiny function like that, it'll inline it). The compiler knows the target system better than you, and can make smart optimization decisions. If you use a macro, you are forcing inlining, which may not be optimal. So from an efficiency perspective, just use a function.

However, there's another negative side of using macros. Macros do not respect scoping rules at all, whereas functions do. You can put a function in a namespace, and if you scope things right, you can have multiple functions, all with the same name (this is important so the code you write doesn't conflict with or "override" the code someone else writes, even if you both have a function named "dotProduct" for example). Macros don't do this.

And again, another negative of using macros: how their arguments are evaluated. Try the following with the macro, and compare it with a function:

// I know you're code isn't doing something like this,
// but it shows another drawback of macros vs functions
// Go to <a href="http://ideone.com/YWAsoX">http://ideone.com/YWAsoX</a> for the results
#include <iostream>

#define MACRO_ABS(p) ((p) < 0 ? -(p) : (p))

template <typename T>
T function_abs(T p)
{
return p < 0 ? -p : p;
}

int main()
{
int i = -100;

std::cout << "macro: " << MACRO_ABS(i++) << std::endl;
std::cout << "i: " << i << std::endl;

i = -100;

std::cout << "function: " << function_abs(i++) << std::endl;
</p><div>    std::cout << "i: " << i << std::endl;</div>
<div>}</div>
<div>

[ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]

### #23xiajia  Members

Posted 23 January 2013 - 08:20 AM

template<typename T>
void XDELETE(T *&p)
{
delete p;
p = NULL;
}

template<class T> class tempClassA;
template<class T> class tempClass
{
private:
friend tempClassA<T>;
T * m_p;
tempClass()
:m_p(NULL)
{
}
~tempClass()
{
XDELETE(m_p);
}
};
template<class T> class tempClassA
{
public:
tempClassA()
:m_a(new tempClass<T>())
{
}
~tempClassA()
{
XDELETE(m_a);
}
private:
tempClass<T> * m_a;
};

int main()
{
tempClassA<int> a;
return 0;
}


compile error.

Edited by xiajia, 23 January 2013 - 09:06 AM.

### #24Álvaro  Members

Posted 23 January 2013 - 08:33 AM

compile error.

What compiler error? If you can't find the time to report the problem correctly and completely, I can't find the time to help you.

### #25Brother Bob  Moderators

Posted 23 January 2013 - 08:34 AM

What error and where is it?

### #26xiajia  Members

Posted 23 January 2013 - 08:50 AM

compiling with VS2005,

error C2248:'tempClass<T>::~tempClass' : cannot access private member declared in class 'tempClass<T>'

line 4: delete p;

Edited by xiajia, 23 January 2013 - 08:51 AM.

### #27rip-off  Moderators

Posted 23 January 2013 - 08:52 AM

Do we have to guess the error? Or the intended behaviour?

Presumably from your "friend" declaration, you want "tempClass" to only be created and destroyed by "tempClassA".

You could clarify this using better naming (these two class names are confusingly similar):


</p><div>template<typename T></div>
<div>void XDELETE(T *&p)</div>
<div>{</div>
<div>    delete p;</div>
<div>    p = nullptr;</div>
<div>}</div>
<div> </div>
<div>template<class T> class Example;</div>
<div> </div>
<div>template<class T> class MostlyPrivate</div>
<div>{</div>
<div>private:</div>
<div>    friend class Example<T>;</div>
<div> </div>
<div>    T * m_p;</div>
<div>    </div>
<div>    MostlyPrivate()</div>
<div>        :m_p(nullptr)</div>
<div>    {</div>
<div>    }</div>
<div>    </div>
<div>    ~MostlyPrivate()</div>
<div>    {</div>
<div>        XDELETE(m_p);</div>
<div>    }</div>
<div>};</div>
<div> </div>
<div>template<class T> class Example</div>
<div>{</div>
<div>public:</div>
<div>    Example()</div>
<div>        :m_a(new MostlyPrivate<T>())</div>
<div>    {</div>
<div>    }</div>
<div>    </div>
<div>    ~Example()</div>
<div>    {</div>
<div>         XDELETE(m_a);</div>
<div>    }</div>
<div>    </div>
<div>private:</div>
<div>    MostlyPrivate<T> * m_a;</div>
<div>};</div>
<div> </div>
<div>int main()</div>
<div>{</div>
<div>    Example<int> a;</div>
<div>    return 0;</div>
<div>}</div>

The compiler error (having corrected some of the more trivial errors) says:

$g++ test.cpp -std=c++0x test.cpp: In function ‘void XDELETE(T*&) [with T = MostlyPrivate<int>]’: test.cpp:38:10: instantiated from ‘Example<T>::~Example() [with T = int]’ test.cpp:47:18: instantiated from here test.cpp:23:5: error: ‘MostlyPrivate<T>::~MostlyPrivate() [with T = int]’ is private test.cpp:4:5: error: within this context Highlighting the most relevant lines:$ g++ test.cpp -std=c++0x
test.cpp: In function ‘void XDELETE(T*&) [with T = MostlyPrivate<int>]’:
test.cpp:38:10:   instantiated from ‘Example<T>::~Example() [with T = int]’
test.cpp:47:18:   instantiated from here
test.cpp:23:5: error: ‘MostlyPrivate<T>::~MostlyPrivate() [with T = int]’ is private
test.cpp:4:5: error: within this context

Essentially, the problem is that XDELETE is trying to call a private destructor. Even though "Example" is a friend, and the call to XDELETE is coming from "Example", that does not confer friendship onto XDELETE. Note this is an interesting case of where the macro version could be considered to be "bypassing" language rules.

One solution is to add XDELETE as a friend too. Another is to consider making the destructor public - perhaps making the constructor private is enough to prevent whatever mishaps you're worried about?

The most obvious solution is to use delete, not XDELETE, because you are in a destructor and thus it would be undefined behaviour for any code to touch this pointer again anyway.

### #28Álvaro  Members

Posted 23 January 2013 - 09:03 AM

Actually, XDELETE is an abomination. In correctly written code, setting the pointer to 0 will never matter, so it's a wasted operation. In poorly written code, it will sometimes help (if someone tries to access a member through the pointer after it has been deleted) and sometimes hurt (if someone tries to delete the pointer again, which is a problem that will now be masked).

You are also missing a semicolon after the tempClassA block, and the word class' after friend'.

Why do you feel the need to call new' at all? I rarely do it. Your members should be objects, not pointer to objects, unless you have a good reason ("I used to be a Java programmer and feel the need to write new' all over the place" is not a good reason).

Edited by Álvaro, 23 January 2013 - 09:07 AM.

### #29xiajia  Members

Posted 23 January 2013 - 09:05 AM

I think "make the destructor public" is better than "add XDELETE as a friend to class MostlyPrivate",

### #30xiajia  Members

Posted 23 January 2013 - 09:11 AM

about the semicolon after the tempClassA,it is a small accident. the main function is added later.The compiler will not complain if not coupled.

### #31xiajia  Members

Posted 23 January 2013 - 09:32 AM

Why do you feel the need to call new' at all? I rarely do it. Your members should be objects, not pointer to objects, unless you have a good reason ("I used to be a Java programmer and feel the need to write new' all over the place" is not a good reason).

do you mean call ‘new’ at the constructor？This is just one case.Not always the case.

Edited by xiajia, 23 January 2013 - 09:36 AM.

### #32Álvaro  Members

Posted 23 January 2013 - 09:48 AM

Why do you feel the need to call new' at all? I rarely do it. Your members should be objects, not pointer to objects, unless you have a good reason ("I used to be a Java programmer and feel the need to write new' all over the place" is not a good reason).

do you mean call ‘new’ at the constructor？This is just one case.Not always the case.

No, I mean what I said. I very rarely use new' in my code. When I do, it's almost always in factory classes.

### #33SiCrane  Moderators

Posted 23 January 2013 - 06:16 PM

I'm not sure why you're objecting in this case. Heap allocating the reference count in a non-intrusive reference counting smart pointer is perfectly normal. Not that there aren't other issues to look at here (such as those already mentioned as well as exception safety).

### #34xiajia  Members

Posted 23 January 2013 - 10:57 PM

I have yet to learn the knowledge of design patterns, I learned the factory pattern to do some more in-depth discussion.

### #35Trienco  Members

Posted 23 January 2013 - 11:13 PM

I guess the objection is simply because with only the latest example code, it boils down to

class MyClass
{
public:
MyClass() : ptr_(new something) {}
~MyClass() { delete ptr_; }
};
`

That on its own is usually not just pointless ("something" should just be a regular member), but also a time bomb just waiting for an instance being accidentally copied (assigning it or passing it to a standard container). So usually whenever you see this kind of pattern and it really is necessary, explicitly making it non-copyable or supplying assignment and copy constructor should become almost a reflex.

Since this is just example code, it most likely doesn't apply, but still, I've encountered production code where the author thought that setting the pointer to NULL in the destructor will somehow magically affect all the other objects that have a copy of that pointer.

f@dzhttp://festini.device-zero.de

### #36xiajia  Members

Posted 24 January 2013 - 01:28 AM

use smart pointer or use reference counting can solve this problem.

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.