Sign in to follow this  
Hurp

Calling new in a constructor, good or bad?

Recommended Posts

Hurp    106
I thought I remembered hearing some where that calling new in a constructor was considered "bad practice". I was wondering if this is true? If it is considered bad practice the only thing I could think of preventing this was to create an "Initialize" function for every class that is dependent on creating new memory. However, then if a person calls just the constructor, his class is not really set up and that could cause problems later if the proper checks are not in place.

Share this post


Link to post
Share on other sites
the_edd    2109
Quote:
Original post by Hurp
I thought I remembered hearing some where that calling new in a constructor was considered "bad practice".


In general, it's not bad practice.

There may however be some reason not to do this on a memory constrained system, or with a compiler that doesn't support exceptions. I can't think what right now, though.

Where did this advice come from, exactly?

Share this post


Link to post
Share on other sites
Antheus    2409
Design perspective:
It is related to testability and reuse. Clean Code talks.

In some ways, creating new "heavy" instances in constructors is more OO-like, where objects knows everything about itself, including how to create all of its dependencies.

Alternate approach is more "procedural" in that objects and functions (aside from top-level ones) never allocate any resources directly, but only manipulate existing ones provided from some external source.

Quote:
However, then if a person calls just the constructor, his class is not really set up and that could cause problems later if the proper checks are not in place.

That is a different problem, it has to do with constructors being unable to signal failure if exceptions are disabled.

Share this post


Link to post
Share on other sites
visitor    643
There may be scenarios where it may not be wise, e.g having many of them in the initializer list:


class X
{
Y* py;
Z* pz;
public:
X(): py(new Y), pz(new Z) {}
//...
}


If the second allocation fails, the first object is inevitably leaked.

But there won't be a problem if you hold pointer members in a smart pointer instead:


class X
{
shared_ptr<Y> py;
shared_ptr<Z> pz;
public:
X(): py(new Y), pz(new Z) {}
//...
}


Share this post


Link to post
Share on other sites
dmail    116
It is difficult to give a hard and fast rule, yet calling new in a constructor could be considered bad practice. If it is possible and relevant then better practice would be creating via a factory and passing in the dependencies to the constructor.

Share this post


Link to post
Share on other sites
DieterVW    724
This doesn't seem to be covered yet in the discussion, but the primary issue here is the possibility of an exception being thrown during object construction - which would cause resources to be leaked since no destructor is called for a class that doesn't fully constructed.

One very simple way to work around this problem is to use smart pointers store heap objects.

example:


struct X
{
void* heapObj;
smart_ptr<void *> safeObj;

X()
{
heapObj = new .....;
safeObj = new .....;
throw std::exception();
}

~X() { delete heapObj; }
};





The safeObj member was fully constructed during the construction of X, so when the stack unwinds, safeObj will call its internal destructor and free the heap allocated memory. However, heapObj will be leaked since X was not fully constructed before throwing, which means that X's destructor is never called.

Any time a constructor could throw, care should be taken to protect heap allocated member pointers otherwise there will be leaks. It's pretty trivial to prevent as there are a lot of great smart pointers available. It may matter less in small home projects, but is very important for any full enterprise software that is expected to be fault tolerant.

Share this post


Link to post
Share on other sites
Antheus    2409
Quote:
Original post by DieterVW
One very simple way to work around this problem is to use smart pointers store heap objects.

Overkill and feature creep. Unless class design requires smart pointers, there is no need to put them in just for this:

struct X
{
Foo* heapObj;
Foo* safeObj;

X()
{
std::auto_ptr<Foo> a = new ...;
std::auto_ptr<Foo> b = new ...;
// we're safe from here on
heapObj = a;
safeObj = b;
}
};


auto_ptr is used for all kinds of exception safety anyway, even where other resources might not have adequate protection. It also doesn't require any third-party libraries.

I find it ironic that when people were trying to fix Python's GIL, adding thread-safe reference counting (among other things) reduced execution speed by a mere factor of 2, which was deemed completely unacceptable, despite Python being notoriously slow.

Yet smart_ptr is being thrown around C++ code like it's candy, even though it is exactly the same thing. It performs one heap allocation per construction, and an interlocked operation per assignment, possibly even more during assignment/copy-construction.

In some distant, yet bright utopian future, unique_ptr will take the role of auto_ptr.

Share this post


Link to post
Share on other sites
DieterVW    724
Anthes' design is fine too, though the example forgets to release() the auto_ptr's hold on the variables before the constructor ends - there by freeing the memory too soon. But that is easily remedied.

It won't be necessary soon to only rely on 3rd party libraries since several different types of smart pointers are being added to the standard library, including the unique_ptr I believe. Aside from that you could easily write your own or copy the code.

The reason that there are several types of smart pointers is to address various different scenarios, one of which is speed. The boost library has some interesting studies on smart pointer design and speed. For instances where objects get shared, intrusive referenced counted objects do very well speed wise. Not that COM is the best design pattern, but the DX API uses embedded reference counting and is a performance sensitive API with much success. Every DX object you make is a smart pointer on the user side and yet no one cares.

Not all smart pointers pay the price of thread safety either. Use the right tool for the right job.

My example is just meant to provide direction for further study by interested parties.

Using smart pointers as members is actually a great solution for shared scenarios. It really depends on what you're goals are and what costs you're willing to pay to make coding and tracking easier. A good smart pointer should be easy to use, no more cumbersome than a normal pointer. Tracking memory is never free, it either costs dev and design time up front, or else you rely on tools that pay some runtime price as you go. But smart pointers do help reduce leaks and reduce dev time - which to many people is important.

Share this post


Link to post
Share on other sites
Antheus    2409
Quote:
Original post by DieterVW
Anthes' design is fine too, though the example forgets to release() the auto_ptr's hold on the variables before the constructor ends - there by freeing the memory too soon. But that is easily remedied.


My bad, should be release(), not just plain assignment.

        heapObj = a.release();
safeObj = b.release();

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this