Why it is not good code for constructor

[George2]

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

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

}

thanks in advance,
George

 

[Kira Yamato]

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

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

}

thanks in advance,
George

If an exception of type Error is thrown inside the constructor's try
block, will the object be fully initialized on return?

 

[Tadeusz B. Kopec]

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

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

}

thanks in advance,
George

If an exception of type Error is thrown inside the constructor's try
block, will the object be fully initialized on return?

The only return from constructor's try block can be via an exception, so
it doesn't matter.

 

[Bob Terwilliger]

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

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

}

thanks in advance,
George

This does an anonymous memory allocation, so how would you ever reclaim
the memory once it's allocated? Whenever you allocate memory with 'new',
you should reclaim the memory with 'delete' once you're done with it.

Bob Terwilliger

 

[Tadeusz B. Kopec]

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

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

}

Whether it's good code for constructor or not depends on what B is. If B
stands for boost::scoped_array the code is absolutely OK. Generally the
problem is: when B's constructor throws what happens with the allocated
memory. But this code suggests that B takes ownership of it so I don't
see problem. If B's constructor throws it should free the memory.

 

[Kira Yamato]

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

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

}

thanks in advance,
George

If an exception of type Error is thrown inside the constructor's try
block, will the object be fully initialized on return?

The only return from constructor's try block can be via an exception, so
it doesn't matter.

No. Had the constructor returned via exception, the object will be
outside the scope of its declaration.

In his code, the constructor catches the exception and returns
normally. His object remains in scope. The question I asked him is if
his object is valid or not.

 

[Kira Yamato]

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

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

}

Whether it's good code for constructor or not depends on what B is. If B
stands for boost::scoped_array the code is absolutely OK. Generally the
problem is: when B's constructor throws what happens with the allocated
memory. But this code suggests that B takes ownership of it so I don't
see problem. If B's constructor throws it should free the memory.

Please do not encourage bad coding practices.

It is not ok. We don't know what "..." can be. If in the middle of
initializing the object as part of the "..." and an exception of
Exception is thrown, then the object may not be fully initialized.

 

[Kira Yamato]

On 2007-12-26 09:10:47 -0500, George2 <[email protected]> said:

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

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

}

thanks in advance,
George

If an exception of type Error is thrown inside the constructor's try
block, will the object be fully initialized on return?

The only return from constructor's try block can be via an exception, so
it doesn't matter.

No. Had the constructor returned via exception, the object will be
outside the scope of its declaration.

In his code, the constructor catches the exception and returns
normally. His object remains in scope. The question I asked him is if
his object is valid or not.

Alright. I retract what I said.

I did not know the behavior of the constructor when an exception is
thrown inside it.

In the OP's original code, the catch block will rethrow the exception
even if it is not explicitly told so.

 

[Tadeusz B. Kopec]

On 2007-12-26 09:10:47 -0500, George2 <[email protected]>
said:

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

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

}

thanks in advance,
George

If an exception of type Error is thrown inside the constructor's try
block, will the object be fully initialized on return?

The only return from constructor's try block can be via an exception,
so it doesn't matter.

No. Had the constructor returned via exception, the object will be
outside the scope of its declaration.

In his code, the constructor catches the exception and returns normally.
His object remains in scope. The question I asked him is if his object
is valid or not.

Sorry, I wasn't precise. You can leave the catch clause of constructor's
function-try-block only by throwing exception. Standard makes absorbing
exception there impossible so he can't return normally. Inside the catch
clause object is invalid and accessing any non-static members or base
classes is UB.

 

[Tadeusz B. Kopec]

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

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

}

Whether it's good code for constructor or not depends on what B is. If
B stands for boost::scoped_array the code is absolutely OK. Generally
the problem is: when B's constructor throws what happens with the
allocated memory. But this code suggests that B takes ownership of it
so I don't see problem. If B's constructor throws it should free the
memory.

Please do not encourage bad coding practices.

It is not ok. We don't know what "..." can be. If in the middle of
initializing the object as part of the "..." and an exception of
Exception is thrown, then the object may not be fully initialized.

Sorry, but I don't understand. What's in constructor's body is completely
irrelevant. Only important thing is what B is. If it means something that
takes ownership of the memory passed to it's constructor, for example a
member of type boost::scoped_array, it's the proper idiom for
initialising. If C's constructor body throws, all members and base
subobjects will be destroyed before reaching catch clause, and as B has
taken the ownership of the memory, it deleted it.
If, on the other hand, B doesn't take ownership of the memory, passing to
its constructor a newly allocated pointer is a big error.

 

[Kira Yamato]

On Wed, 26 Dec 2007 06:10:47 -0800, George2 wrote:

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

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

}

Whether it's good code for constructor or not depends on what B is. If
B stands for boost::scoped_array the code is absolutely OK. Generally
the problem is: when B's constructor throws what happens with the
allocated memory. But this code suggests that B takes ownership of it
so I don't see problem. If B's constructor throws it should free the
memory.

Please do not encourage bad coding practices.

It is not ok. We don't know what "..." can be. If in the middle of
initializing the object as part of the "..." and an exception of
Exception is thrown, then the object may not be fully initialized.

Sorry, but I don't understand. What's in constructor's body is completely
irrelevant. Only important thing is what B is. If it means something that
takes ownership of the memory passed to it's constructor, for example a
member of type boost::scoped_array, it's the proper idiom for
initialising. If C's constructor body throws, all members and base
subobjects will be destroyed before reaching catch clause, and as B has
taken the ownership of the memory, it deleted it.
If, on the other hand, B doesn't take ownership of the memory, passing to
its constructor a newly allocated pointer is a big error.

You're right. I didn't really understand this behavior of the
constructor when an exception is thrown in it. I retract my statement
earlier.

Sorry.