Array class member initialization in C++

Question

I have the following code snippet:

#include <iostream>
using namespace std;
class A {
    int* data;
    int size;
public:
    A(int s):size(s)
    {
        data = new int[size];
    }
    A() {
        data = nullptr;
    }
    ~A() {
        if (data) delete [] data;
    }
};
class B {
    A a[2];
public:
    B() {
        a[0] = A(10);
        a[1] = A(11); 
    }
};
int main(int argc, char *argv[]) {
    B b;
}

In the C++ code above, I have class A which has an array member int* data, and the (de)allocation of memory are handled by (de)constructor. The I created class B which has an array of class A of fixed length as a data member.

My question is: how to elegantly initialise the member A a[2]? In the code above, the A(10) and A(11) are created on the stack, when jumping out of the scope, their destructors will be called, hence the data comes invalid. When jumping of the main function's scope, the pointers held by a[2] will be deallocated twice, causing the error:

pointer being freed was not allocated.

One possible solution is to carefully design a copy constructor and a move constructor, by doing so the above coding paradigm could work.

Another solution I've tried is to initialise the array in the initialization list of class B:

B() : a { A(10), A(11) }

This solution works and I don't really tell the underlying mechanism of initialization list. I think it must be quite different from simply construct and copy. I really expected some experts could give an elaborate explanation of this mechanism. Of course, this solution is ugly hard-coded and not flexible.

So I wonder if there are some programming paradigms in C++ to tackle this design problem?

Before we jump into any complicated stuff, how about understanding if (data) delete [] data; in depth -- have you looked up what delete [] does? — Kerrek SB
– Kerrek SB, Commented Apr 26, 2016 at 9:27
For a paradigm, see the rule of five. For a solution, see std::vector. — Quentin
– Quentin, Commented Apr 26, 2016 at 9:29

Community · Accepted Answer · 2020-06-20 09:12:55Z

In the code above, the A(10) and A(11) are created on the stack

They are temporary objects. It is not specified where they are created or if they're created at all.

when jumping out of the scope, their destructors will be called

The destructor of each temporary will be called after the corresponding move assignment statement ends.

One possible solution is to carefully design a copy constructor and a move constructor, by doing so the above coding paradigm could work.

And {copy,move} assignment operator too. You should always do that when the implicitly declared ones don't do the right thing. And they never do the right thing if you delete something in the destructor.

Another solution I've tried is to initialise the array in the initialization list of class B

This solution works and I don't really tell the underlying mechanism of initialization list. I think it must be quite different from simply construct and copy.

The bug in the original code is badly behaving move assignment operator of A. Since the initialization list never move assigns from a temporary, it never triggers the bug.

This is actually the more elegant way to construct a that you asked for. Not because it avoids the bug, but because avoiding unnecessary moving is good thing, intrinsically.

So I wonder if there are some programming paradigms in C++ to tackle this design problem?

Yes. RAII and Single responsibility principle. Unless your class does nothing else, besides managing the memory pointed by data, it should not be managing the memory. Instead, it should delegate the memory management to a RAII object. In this case, you should use a std::vector member.

class A {
    std::vector<int> data;
public:
    A(int s):data(s) {}
    A() = default;
};

marom · Accepted Answer · 2016-04-26 09:35:17Z

1

Using an initializer list to construct B::a, like this:

class B {
    A a[2];
public:
    B() : a({10, 11}){
    }
};

answered Apr 26, 2016 at 9:35

marom

5,23813 silver badges15 bronze badges

Comments

anatolyg · Accepted Answer · 2016-04-26 10:06:35Z

1

The ideal answer would be to force A to use movements instead of copies, or on a copy to allocate new space for the item. Of the two, the most efficient is the former and so I will expand on it below:

Forcing movement can be done in two fashions:

Delete the copy constructor and copy operator=, and implement your own move constructor and operator=
Consistently use std::move and std::swap.

Of these, the former is superior in that you will be unable to accidentally copy the class, but with the latter the fact that you are moving will be more evident.

To delete the default copy methods do:

class A {
    A( const A& a ) = delete;
    A& operator =( const A& a ) = delete;
}

edited Apr 26, 2016 at 10:06

anatolyg

28.5k9 gold badges66 silver badges148 bronze badges

answered Apr 26, 2016 at 9:52

Nonanon

5603 silver badges10 bronze badges

2 Comments

anatolyg Over a year ago

I edited your answer, I hope it's better now (please revert if you think otherwise). Also, as far as I remember, declaring move-operations (constructor and/or assignment operator) automatically deletes the copy ones - am I right?

Nonanon Over a year ago

@anatolyg thanks for the edit, and I am unsure if the declaration does delete the copy methods. I would personally still delete the copy methods so that my intentions are explicit

Collectives™ on Stack Overflow

Array class member initialization in C++

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related