1

I have class NameClass with members:

class NameClass {
    public:

    int count;
    apple* array;
} 

and constructor of this:

NameClass::NameClass()
{
    count = 12;
    array = new apple[count];
    ...
}

Should I delete this array in destructor of class? And how?

This makes the error ("pointer being freed was not allocated"):

delete []apple;
or 
delete apple;
πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
Vlad
  • 1,541
  • 1
  • 21
  • 27
  • @chris You know that it's a homework. – mostruash Nov 21 '14 at 00:34
  • `delete []array;` is correct (`delete []apple;` doesn't make sense and should not have compiled, if apple is a class, as the code suggests). If this you still get the error, the particular constructor you showed here was not executed or `array` was set to something else during the lifetime of the object. Maybe, there is another constructor that *doesn't* allocate the array? Maybe, you have a copy constructor/ copy assignment operator that does not do a deep copy, so you are deleting the array twice (in two objects)? We can only guess... – Oguk Nov 21 '14 at 00:38
  • @Oguk, constructor only one and no operators. – Vlad Nov 21 '14 at 00:41
  • @mostruash _"You know that it's a homework."_ We don't care usually, unless the OP correctly state their requirements and restrictions clearly. – πάντα ῥεῖ Nov 21 '14 at 00:42
  • 1
    You'll also need to obey the Rule of Three. – aschepler Nov 21 '14 at 00:44
  • @Vladislav That's exactly what I mean. If they are not defined but you *do* a copy construction or an assignment, you will only have a shallow copy, resulting in the array being deleted twice. – Oguk Nov 21 '14 at 00:47

2 Answers2

4

delete []array; should be the correct delete operation used in the destructor.

If you get an error as mentioned

"This makes the error ("pointer being freed was not allocated"):"

you either missed to initialize array correctly with a nullptr (or NULL), or you passed an address of non allocated memory to it (like a stack allocated reference).

πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
  • @Vladislav Note exactly what I'm saying (updated), Xcode doesn't make a difference IMHO. – πάντα ῥεῖ Nov 21 '14 at 00:38
  • I wrote this `array = nullptr; delete array;` in destructor and it works, but I'm not sure that it's correct. – Vlad Nov 21 '14 at 00:42
  • 1
    @Vladislav _"I wrote this `array = nullptr; delete array;`"_ That should always work. Though if you're using `new []` to allocate, you'll need `delete[]` as pendant. Just `delete` leads to undefined behavior. – πάντα ῥεῖ Nov 21 '14 at 00:45
  • 2
    @Vladislav `array = nullptr; delete array;` is not what you want. It suppresses the error by making the `delete` a no-op, but then your destructor will never actually delete the array `new`ed in the constructor. – Oguk Nov 21 '14 at 00:56
  • @Vladislav Even more drastically spoken: If you do that, you can just as well remove the line `delete []array;` in its entirety. This will suppress the error in the same way, but will not solve anything. – Oguk Nov 21 '14 at 01:03
1

delete []apple; wouldn't compile, but you obviously have a runtime error, so you must have delete []array;. First of all, this is correct!

Assuming your class is next to trivial, the reason for the error could be that two of your NameClass objects have an array pointer that points to the same array. This can happen if your class does not have a user-defined copy constructor and/or assignment operator, in which case the compiler will generate one for you. If you then do something like this

NameClass n1;
NameClass n2 = n1;

or

NameClass n1;
NameClass n2;
n2 = n1;

in your code, the compiler will just assign all the members (including the array pointer) to each other, making array point to the same piece of memory in both objects (and, in the latter case, cause a memory leak for the memory initially allocated in the constructor of n2). What you need to do is to define the copy constructor and assignment operator such that they make a copy of the array pointed to by array. Because the things you do in the destructor, in the copy constructor and in the copy assignment operator (and in the constructor, but that is obvious) have to be compatible to each other, this is called the Rule of Three.

Community
  • 1
  • 1
Oguk
  • 1,127
  • 7
  • 13