Is this C++ destructor redundant?

Question

I've received some C++ code with various structures defined like this:

typedef struct _someStruct_ {
   std::string someString; 
   std::vector<std::string> someVectorOfStrings;
   int  someOtherStuff;

   ~_someStruct_()
   {
      someString.clear();
      someVectorOfStrings.clear(); 
   }
} someStruct;

Is the destructor here completely redundant - if the structure were to be destructed by the default destructor, won't any strings, vectors, etc. be destructed anyway?

If I'd written the code, I wouldn't have thought of adding an explicit destructor here - I'd just let the compiler get on with it.

As I understand it, the only time you might need to create your own destructor in a struct is if any of the members of the structures contain pointers to data that might need cleaning up, or if some extra functionality (e.g. for debugging, logging when a structure gets deleted) is needed.

Am I missing something here - is there any reason why strings and vectors have been explicitly cleared in the destructor? My suspicion is that the person who sent me this is really a C programmer (cf. the typedef) who's tried to turn some C code into C++.

Yes, its completely useless. However if the vector had been holding raw pointers you'd have to call delete yourself on each of the pointers (at which point you should use smart pointers). — Borgleader, Oct 03 '14 at 14:36
as long as the destructors of the members do what is needed, there's no need for a custom destructor. — Karoly Horvath, Oct 03 '14 at 14:38
`typedef _someStruct_ { ... } someStruct;` is also completely redundant, just write `struct someStruct { ... };`. Since the type cannot be valid in a C program (because it uses a destructor) there is no reason whatsoever to use a typedef for the struct name. — Jonathan Wakely, Oct 03 '14 at 15:54
If one of answers answered your question, mark it as accepted. — GingerPlusPlus, Oct 04 '14 at 13:36
Btw this destructor changes order of cleaning string and vector. — magras, Oct 19 '14 at 12:16

Konrad Rudolph · Accepted Answer · 2014-10-04T14:49:36.030

71

Yes, the destructor is completely redundant.

As you’ve said yourself the code has other warnings signs. Using typedef struct for instance makes no sense whatsoever in C++, it’s as redundant as the empty destructor: The code was written by somebody with a marginal grasp of C++, there are bound to be more gotchas (for one thing, the class name is invalid due to the leading underscore in global scope).

edited Oct 04 '14 at 14:49

answered Oct 03 '14 at 14:37

Konrad Rudolph

530,221
131
937
1,214

3

Wait, doesn't that have to be a leading underscore *followed by an uppercase character*? – Bartek Banachewicz Oct 03 '14 at 15:51
@JonathanWakely Oh damn, now I get it. My bad. – Bartek Banachewicz Oct 03 '14 at 15:53
`typedef struct tagname {} type` is very common in headers that have to be both C and C++ compatible respect to pre 1996 compilers (mostly due to the fact that C struct tags did not share the type namespace, but C++ does) The entire `` included in `` have large use of this technique that allow those definitions to work untouched since 1994 up to nowadays. A very good example of the "don't repeat yourself" principle. Don't be so cruel in moral judgement. – Emilio Garavaglia Oct 04 '14 at 17:21
4

@Emilio That’s simply not applicable here since the code is not valid C code anyway. – Konrad Rudolph Oct 04 '14 at 18:09

score 52 · Answer 2 · answered Oct 03 '14 at 14:39

52

In fact, this is worse than simply using the implicit destructor.

By having the explicit destructor, the compiler will not provide an implicit move constructor for you!

answered Oct 03 '14 at 14:39

Bill Lynch

80,138
16
128
173

score 21 · Answer 3 · answered Oct 04 '14 at 01:17

The destructor is almost completely redundant.

It does three things.

First, it blocks the automatic creation of copy/move constructors and assignment. This ... may not be a good thing. But maybe it is desired. It is, however, not the same as not being there.

Second, it changes the order the stuff is cleaned up in. The buffer held by the string is cleared, then each buffer held by the strings in the vector are destroyed while the string holding them are destroyed, then the vector with a buffer of unused memory is destroyed (returning the memory), then the now empty string is destroyed.

With a default destructor, the order is the vector's string's buffers are destroyed, then the memory for the vector's strings is returned while the vector is destroyed, then the string is destroyed along with its buffer being returned.

You could possible detect this with suitable overloads to operator new & delete, or if you used custom allocators.

Finally, such destructors can sometimes be easier to debug, as you can step through them.

The odds are, however, that none of these subtle effects where intended by the developer who wrote that code.

It may do to think about the mechanics, the default destructor may deallocate but not "scrub". In fact I'm not sure even that erase and clear will explicitly zero excess positions. We may not want memory scraped, even unallocated memory. — mckenzm, Mar 21 '15 at 20:39

Is this C++ destructor redundant?

3 Answers3