Jump to content

  • Log In with Google      Sign In   
  • 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.

  • You cannot reply to this topic
35 replies to this topic

#21 xiajia   Members   -  Reputation: 163

Like
1Likes
Like

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"?



Sponsor:

#22 Cornstalks   Crossbones+   -  Reputation: 6991

Like
0Likes
Like

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 ]

#23 xiajia   Members   -  Reputation: 163

Like
0Likes
Like

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. sad.png


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


#24 Álvaro   Crossbones+   -  Reputation: 13897

Like
0Likes
Like

Posted 23 January 2013 - 08:33 AM

compile error. sad.png

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.

#25 Brother Bob   Moderators   -  Reputation: 8575

Like
0Likes
Like

Posted 23 January 2013 - 08:34 AM

What error and where is it?



#26 xiajia   Members   -  Reputation: 163

Like
0Likes
Like

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.


#27 rip-off   Moderators   -  Reputation: 8689

Like
0Likes
Like

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   Crossbones+   -  Reputation: 13897

Like
0Likes
Like

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.


#29 xiajia   Members   -  Reputation: 163

Like
0Likes
Like

Posted 23 January 2013 - 09:05 AM

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



#30 xiajia   Members   -  Reputation: 163

Like
0Likes
Like

Posted 23 January 2013 - 09:11 AM

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



#31 xiajia   Members   -  Reputation: 163

Like
0Likes
Like

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   Crossbones+   -  Reputation: 13897

Like
0Likes
Like

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.

#33 SiCrane   Moderators   -  Reputation: 9662

Like
0Likes
Like

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).

#34 xiajia   Members   -  Reputation: 163

Like
0Likes
Like

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.



#35 Trienco   Crossbones+   -  Reputation: 2224

Like
0Likes
Like

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

#36 xiajia   Members   -  Reputation: 163

Like
0Likes
Like

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.



PARTNERS