7

Why one would want to explicitly clear the a vector member variable (of on in a dtor (please see the code below). what are the benefits of clearing the vector, even though it will be destroyed just after the last line of dtor code will get executed?

class A
{
~A()
{
   values.clear();
}

private: 
  std::vector < double > values_;
};

similar question about a the following code:

class B
{
~B()
{
   if (NULL != p)
   {
       delete p_;
       p_ = NULL;
   }
}

private: 
  A * p_;
};

Since there is no way the dtor will get called twice, why to nullify p_ then?

Byron Whitlock
  • 52,691
  • 28
  • 123
  • 168
Michael
  • 177
  • 3

3 Answers3

11

In class A, there is absolutely no reason to .clear() the vector-type member variable in the destructor. The vector destructor will .clear() the vector when it is called.

In class B, the cleanup code can simply be written as:

delete p_;

There is no need to test whether p_ != NULL first because delete NULL; is defined to be a no-op. There is also no need to set p_ = NULL after you've deleted it because p_ can no longer be legitimately accessed after the object of which it is a member is destroyed.

That said, you should rarely need to use delete in C++ code. You should prefer to use Scope-Bound Resource Management (SBRM, also called Resource Acquisition Is Initialization) to manage resource lifetimes automatically.

In this case, you could use a smart pointer. boost::scoped_ptr and std::unique_ptr (from C++0x) are both good choices. Neither of them should have any overhead compared to using a raw pointer. In addition, they both suppress generation of the implicitly declared copy constructor and copy assignment operator, which is usually what you want when you have a member variable that is a pointer to a dynamically allocated object.

James McNellis
  • 348,265
  • 75
  • 913
  • 977
  • 4
    Nice answer except for the "rarely use delete in C++" code. I agree that things like auto_ptr and unique_ptr take care of many situations it is absolutely false to say that "delete" should be /rarely/ used. – edA-qa mort-ora-y Nov 21 '10 at 06:50
  • 5
    @edA: No, not really. It should rarely be used. Give us a situation where you think you need it, we'll make it cleaner and safer. **All** resources should be wrapped, so you only have to use the wrapper. If you've got a `delete` anywhere aside from a destructor, you've done it wrong – GManNickG Nov 21 '10 at 06:52
  • 5
    @edA: I stand by that statement. I have not written one `delete` expression in new production code in the last two years (before then I was a complete noob when it came to writing correct C++ code). – James McNellis Nov 21 '10 at 06:54
  • 1
    @edA: I second James McNellis's and GMan's comment. Calling `delete` should only be used if absolutely necessary, and if you write proper C++ code in the first place those times when it is necessary will be very rare. – In silico Nov 21 '10 at 07:12
  • 1
    What if you're the one writing the container? Or, say, an operating system driver? – Crashworks Nov 21 '10 at 07:34
  • 4
    @Crashworks: Note that in my answer I did not say that "you should never need to use `delete`;" I said that "you should **rarely** need to use `delete`." I intentionally chose the word **rarely**. There is rarely a need to write your own resource-owning container (often, you can implement your own container by composing more primitive containers). In the category of "all C++ code," there is far less code written for device drivers than there is code written for higher-level libraries, applications, and other such things (that is, I'd consider driver code rare as well). – James McNellis Nov 21 '10 at 07:37
  • If you're writing your own containers you can't really avoid using a "delete" operator. In many cases the STL containers are simply not efficient compared to a tailored alternative. Also, I consider calling "reset" on a wrapper to be the same as calling "delete" (the semantics are the same). Or do guys also never call "reset" either? – edA-qa mort-ora-y Nov 21 '10 at 09:10
  • @edA: Note that in my answer I did not say that "you should never need to use `delete`;" I said that "you should **rarely** need to use `delete`." I intentionally chose the word **rarely**. (I have this strange feeling that I just said that). Still, while it's sometimes necessary to implement your own container, often (though not always) you can take advantage of existing resource-owning types to do the resource management for you. As for `reset()` and `delete`, they have entirely different semantics: one resets the state of an object, the other destroys an object. – James McNellis Nov 21 '10 at 09:21
  • `reset` and `delete` both delete an object. I don't find it valuable to hide what is happening from users. Hiding things is what leads to very inefficient code. Now, I'll give you that in most cases calling `reset` may be safer than doing a manual `delete`, but for things like auto_ptr the semantics are the same. BTW, I'm not disagreeing with the basic aversion to `delete`, I'm disagreeing with the word *rarely*. – edA-qa mort-ora-y Nov 21 '10 at 09:27
  • 2
    @eda: Users should *never* call `delete`, because in the face of exceptions, that is a recipe for resource leaks. RAII is an integral part of C++ which **does not cost you anything** compared to manual deletes, but it is a lot safer. Implicit cleanup by RAII is a *good thing*, users shouldn't be bothered to do it by hand (because they *will* get it wrong sooner or later). – fredoverflow Nov 21 '10 at 10:52
  • @edA-qa mort-ora-y: the semantics aren't the same even for `auto_ptr`. `sp.reset()` is at least equal to `delete p; p = NULL;` - – MSalters Nov 22 '10 at 10:49
2

In your second example there's no reason to set p_ to null whatsoever, specifically because it is done in the destructor, meaning that the lifetime of p_ will end immediately after that.

Moreover, there's no point in comparing p_ to null before calling delete, since delete expression performs this check internally. In your specific artificial example, the destructor should simply contain delete p_ and noting else. No if, no setting p_ to null.

AnT stands with Russia
  • 312,472
  • 42
  • 525
  • 765
0

In the case of p_, there is no need to set it equal to null in the destructor, but it can be a useful defensive mechanism. Imagine a case where you have a bug and something still holds a pointer to a B object after it has been deleted:

If it trys to delete the B object, p_ being null will cause that second delete to be innocuous rather than a heap corruptor.

If it trys to call a method on the B object, p_ being null will cause a crash immediately. If p_ is still the old value, the results are undefined and it may be hard to track down the cause of the resulting crash.

Steve Rowe
  • 19,411
  • 9
  • 51
  • 82
  • That shouldn't ever be a problem. If multiple things are referencing something, that something should have been put into a `shared_ptr`. Problem solved. – GManNickG Nov 21 '10 at 07:42
  • 2
    If you really have a bug, setting the pointer to null will hide the bug. Is that really what you want? No, you *want* it to crash horribly as soon as possible. – fredoverflow Nov 21 '10 at 10:53
  • @Fred, yes, but leaving it set won't cause that. Have you tried to track down a double delete bug? They aren't fun. – Steve Rowe Nov 21 '10 at 16:28
  • 1
    Tracking down double delete is trivial with a good IDE. For example, Visual Studio reacts with an error message "Debug Assertion Failed! [...] _BLOCK_TYPE_IS_VALID(...)". Then it takes you straight to the `delete` statement. Easy as pie. – fredoverflow Nov 21 '10 at 16:32