Thread: Why it is not good code for constructor

Hello everyone,


Here is a sample from Dr. Dobb C++. In the analysis, the code is bad below.

But I do not think the code is bad,

1. if bad_alloc is thrown in new int[], we just catch it and write some log;
2. if there are any exception in B's constructor, we will also be in catch block and we could also write some log.

Why it is bad code? Any comments?

(I do not agree that there is resource leak, since if we met with bad_alloc in new int[], there is no memory allocated at all, so no root of memory/resource leak).


http://www.ddj.com/cpp/184401297

Code:

C::C(int)
try
   : B(new int[n]) // horrible!
{
   ...
}
catch(Error &e)
{
   
}

thanks in advance,
George

Sorry laserlight,


I do not fully agree. If instance B is constructed correctly, and in other part of constructor of class C, there is an exception, B's destructor will be invoked and in B's destructor, we could free the memory using delete[].

Any comments? If I am wrong, please feel free to correct me.

Originally Posted by laserlight

I think you are correct, except that this assumes that the constructor body itself does not have code that can throw an exception. If the "..." code does throw an exception, the situation would be such that there cannot be a corresponding delete[], which is a Bad Thing.

regards,
George

Hi laserlight,


Why it is horrible? Any potential issues, like memory leak?

Originally Posted by laserlight

I think that's correct, but it proves the point: 'whenever possible use "resource acquisition is initialization"'. Notice that the recommendation is that "if you must acquire resources in another fashion, don't do it in the constructor initializer" (emphasis mine).

If you think that the example is a poor one, just add another pointer member to C, and then use new int[n] to initialise it. In such a case RAII is not used, so the "horrible!" comment surely applies.

regards,
George

Yes, laserlight. :-)


And I think in the thread you pointed out, there is no memory leak. :-)

Originally Posted by laserlight

Yes. It pretty much is related to your other thread on Challenging GotW 66's moral.

regards,
George

Thanks laserlight,


I agree. I have decided to change the function try block to local try block. :-)

Originally Posted by laserlight

... if we are lucky.

regards,
George