Delete a member array correctly in C++

Question

In the class that I am in, refactoring the destructor does not manage destroying its array.

    class MyClass{
    public:
        double a;
        double rect[4];

        MyClass();
        ~MyClass();
    };

    MyClass::MyClass() : a(123.0)
    {
        memset(rect, 0, 4 * sizeof(double));
    }

    MyClass::~MyClass() {}

How do I destroy it correctly? Is a simple delete enough or do I also need to set the following afterwards?

delete[] rect;
*rect= NULL;

It seems you are learning C++ from a subpar source (any decent source would have made you sharply aware of how this works in C++ by the time it introduces `delete`). If you are interested we keep [a curated list of nice books](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). — R. Martinho Fernandes, Aug 20 '13 at 13:25
And while I'm on it, I would advise against `memset` as it is needlessly error-prone; you should be using `std::fill` with `std::begin` and `std::end` (if your compiler is not up-to-date, these last two are easy to implement: https://gist.github.com/rmartinho/3959946). — R. Martinho Fernandes, Aug 20 '13 at 13:29
What @R.MartinhoFernandes said. But in this case, filling the array with zeros in the body of the constructor isn't even needed. `MyClass::MyClass() : a(123.0), rect() {}` will do nicely. — juanchopanza, Aug 20 '13 at 13:37
@MartinoFernandes: Thank you for the advice. I'm using VC11, but this code is very old. — alex555, Aug 20 '13 at 13:37

David Rodríguez - dribeas · Accepted Answer · 2013-08-21T02:32:43.780

15

As a general rule of thumb, you need to delete what you new-ed. The rect array has the same storage as the enclosing object, so it will be taken care by the process of deleting/leaving the scope where the whole object was allocated.

edited Aug 21 '13 at 02:32

answered Aug 20 '13 at 13:21

David Rodríguez - dribeas

204,818
23
294
489

2

"automatic storage" means, roughly, stack-allocated. The `rect` member of a `MyClass` object has the same storage class as the `MyClass` object itself -- which depends on how the `MyClass` object was created. But your main point is correct: destroying the enclosing object will take care of destroying the `rect` member. – Keith Thompson Aug 21 '13 at 00:56

score 2 · Answer 2 · edited Aug 24 '13 at 23:19

2

You need not do anything as it is allocated on automatic storage rather than dynamic memory. Just a general rule:

If you have used new to allocate memory, you need to use delete.
If you have used malloc(), calloc(), realloc() then you may use free(). (Always try using new and delete in C++.)

edited Aug 24 '13 at 23:19

Peter Mortensen

30,738
21
105
131

answered Aug 20 '13 at 13:31

Saksham

9,037
7
45
73

1

It's not allocated on the stack unless it is allocated on the stack. – R. Martinho Fernandes Aug 20 '13 at 13:33
1

http://coliru.stacked-crooked.com/view?id=a3b1d94b9b66f899824da862e468c630-d6c803482d553f395422ddf5510f4bb0 – R. Martinho Fernandes Aug 20 '13 at 13:36
@R.MartinhoFernandes so it should be automatic storage and dynamic memory?? correct?? and not stack and heap – Saksham Aug 20 '13 at 13:39
1

That is better, and I think it's not necessary to be more pedantic (to be excruciatingly accurate would be more confusing than helpful IMO) :) The important bit is that it is not at all about the location but about the duration, and thinking of it in terms of location is misleading (so I am all for expunging the words "stack" and "heap" from discussions on this matter). – R. Martinho Fernandes Aug 20 '13 at 13:42

score 1 · Answer 3 · edited Aug 24 '13 at 23:18

1

You don't have to destroy it, because you haven't created it by your own. As you don't get its memory by malloc() or new, you don't have to free() or delete it. It's an automatic identifier. It's just on the automatic memory. That means it's freed/deleted with the process itself anyway.

edited Aug 24 '13 at 23:18

Peter Mortensen

30,738
21
105
131

answered Aug 20 '13 at 13:23

dhein

6,431
4
42
74

As I mentioned in another answer, the stack is irrelevant (and it's actually a lie to claim the array is on the stack: http://coliru.stacked-crooked.com/view?id=a3b1d94b9b66f899824da862e468c630-d6c803482d553f395422ddf5510f4bb0) – R. Martinho Fernandes Aug 20 '13 at 13:47
@ R. Martinho Fernandes well, I'm sorry about publishing wrong knowledge. But I didn't knew it better. could you explain it to me plz? My point of knew was, all thats allocated automaticly is stored in the stack. But probabl thats wrong, as you guide me to correct it. – dhein Aug 20 '13 at 13:55
2

It's simple. Just forget about the location and focus on the lifetime (the automatic vs dynamic part). The location does not dictate anything here. You can have stuff like that array on the heap and still don't have to deallocate it yourself, and you can also have stuff on the stack that you have to deallocate yourself. – R. Martinho Fernandes Aug 20 '13 at 14:03

Delete a member array correctly in C++

3 Answers3