views:

383

answers:

6

Hey guys,

I'm currently working on a C++ project, where dynamic arrays often appear. I was wondering, what could be the correct way to initialize a dynamic array using the new-operator? A colleague of mine told me that it's a no-no to use new within the constructor, since a constructor is a construct that shouldn't be prone to errors or shouldn't fail at all, respectively. Now let's consider the following example: We have two classes, a more or less complex class State and a class StateContainer, which should be self-explained.

class State {
private:
 unsigned smth;
public:
 State();
 State( unsigned s );
};

class StateContainer {
private:
 unsigned long nStates;
 State *states;
public:
 StateContainer();
 StateContainer( unsigned long n );
 virtual ~StateContainer();
};

StateContainer::StateContainer() {
 nStates = SOME_DEFINE_N_STATES;
 states = new State[nStates];
 if ( !states ) {
  // Error handling
 }
}

StateContainer::StateContainer( unsigned long n ) {
 nStates = n;
 try {
  states = new State[nStates]
 } catch ( std::bad_alloc &e ) {
  // Error handling
 }
}

StateContainer::~StateContainer() {
 if ( states ) {
  delete[] states;
  states = 0;
 }
}

Now actually, I have two questions:

1.) Is it ok, to call new within a constructor, or is it better to create an extra init()-Method for the State-Array and why?

2.) Whats the best way to check if new succeeded:

if (!ptr) std::cerr << "new failed."

or

try { /*new*/ } catch (std::bad_alloc) { /*handling*/ } 

3.) Ok its three questions ;o) Under the hood, new does some sort of

ptr = (Struct *)malloc(N*sizeof(Struct));

And then call the constructor, right?

+6  A: 

You should let the std::bad_alloc propagate - there's likely nothing reasonable you could do anyway.

First of all, throwing an exception from the constructor is the only reliable way to signal a problem - if there's no exception it means the object is completely constructed. So catching std::bad_alloc alone will not help against other possible exceptions.

Then what can you do to "handle" it in such a way that the other code is aware and can react appropriately?

Use exceptions right - let them propagate to the site where they can be reasonably handled.

sharptooth
A: 

Your friend is somewhat right. However, it is common practice to do memory reservations in the Constructor, and deallocations in the Destructor.

The problem with methods that may fail in Constructors is as follows: The constructor has no traditional means of notifying the caller of the problem. The caller expects to get an object instance that is not corrupt in any way.

The solution to this problem is to throw/propagate an exception. The exception will always get through and can be handled by the caller.

ypnos
Your answer is contradictory; you say there is no traditional way of notifying the user of an error then go on to describe one.
Terry Mahaffey
His friend is wrong, with a capital W. End of story.
jalf
Exceptions being so unpopular, most C++ only use them when they must. In their "traditional" thinking there is no place for exceptions and that's why they don't see them as a way to notify the user.
ypnos
+1  A: 

Not a complete answer, just my 2 cents:

1: I would use new in the constructor, although for dynamic arrays, STL is the way to go.

2: the normal error-handling for new is to raise an exception, so checking the returned pointer is useless.

3: don't forget the new-operator to make the story a little bit more interesting.

stefaanv
+5  A: 

The entire purpose of a constructor is to construct an object. That includes initialization. When the constructor finishes executing, the object should be ready to use. That means the constructor should perform any initialization that is necessary.

What your friend suggests leads to unmaintainable and error-prone code, and goes against every good C++ practice. He is wrong.

As for your dynamic arrays, use std::vector instead. And to initialize it, simply pass a parameter to the constructor:

std::vector<int>(10, 20)

will create a vector of 10 integers, all of them initialized to the value 20.

jalf
What do you mean by "includes"? To me, *construction* and *initialization* are synonyms.
FredOverflow
A: 
  1. If you are looking out for a return type i.e. if the function has to return a status then use the separate function (init()) to allocate memory.

    If you check are going to check whether the memory got allocated by checking the NULL condition in all the member functions then allocate memory in the constructor itself.

  2. The exception handling (i.e. try...catch) is a better choice.

  3. Apart from calling the constructor the "this" pointer is also initialized.

Ramakrishna
+1  A: 

Short answer:
No, your friend is wrong. The constructor is where you do allocation + initialization. We even have a term called "Resource Acquisition is Initialization" (RAII)... classes acquire resources as part of their initialization in the constructor and classes free those acquired resources in their destructors.

Long answer:

Consider the following code in a constructor:

member1  = new whatever1[n];
member2  = new whatever2[m];

Now, suppose in the above that the second allocation were to throw an exception, either because the construction of whatever2 failed and threw an exception, or the allocation failed and threw std::bad_alloc. The result is that the memory that you've allocated for whatever1 will have been leaked.

For this reason, it is better to use a container class such as std::vector:

MyClass::MyClass(std::size_t n, std::size_t m) : member1(n), member2(m) {}
// where member1 and member2 are of type std::vector

When using type std::vector, if the second allocation fails, the destructor of the previous std::vector will be invoked, causing the resources to be appropriately released. This is probably what your friend meant when he said you shouldn't be using new (you should be using a container class, instead), in which case he would be mostly correct... although there are smart pointer classes such as boost::shared_ptr which provide you with these same safety guarantees and where you would still need to use new, so he still isn't quite right.

Note that if you have only one object/array that you are allocating, then this is not an issue... which is the way things are in your code... you don't have to worry about leaks due to some other allocation failing. Also, I should add that new will either succeed or throw an exception; it will not return NULL, and so that check is pointless. The only instance where you should catch std::bad_alloc is in the case where you have been forced to do several allocations (you are forbidden from using std::vector), in which case you deallocate the other resources in the handler, but then you rethrow the exception, because you should let it propagate.

Michael Aaron Safyan