Clean up after exception thrown in constructor

[Philipp]

Hello

I have a class hierarchy where the constructor of a child class ("Ford")
can throw an exception, while the constructor of the parent class
("Car") has some side-effects.
If construction fails due to the Exception on the child class being
thrown, the side-effect of the parent constructor cannot be cleaned up.

What would be a better approach to this problem, so that if construction
fails for whatever reason, the system is left in a pristine state?

Thanks for your advice
Phil

Example SSCCE:
package exceptionInCtor;

import java.util.ArrayList;
import java.util.List;

public class Car {
private static List<String> nameList = new ArrayList<String>();
private String name;

public Car(String name){
nameList.add(name); // name is added to list as side-effect
this.name = name;
}

public void remove(){ // called when car is no longer used
nameList.remove(name);
}

public static class Ford extends Car{
public Ford(String name, int wheels){
super(name);
if(wheels > 4){
throw new IllegalArgumentException("Too many wheels");
}
}
}


public static void main(String[] args) {
Car c = null;
try{
c = new Ford("myFord", 5);
} catch (Exception e) {
// cannot call c.remove() here!
e.printStackTrace();
}

System.out.println(c); // prints null
System.out.println(nameList); // prints [myFord], should be empty
}
}

 

[Hendrik Maryns]

Philipp schreef:

Hello

I have a class hierarchy where the constructor of a child class ("Ford")
can throw an exception, while the constructor of the parent class
("Car") has some side-effects.
If construction fails due to the Exception on the child class being
thrown, the side-effect of the parent constructor cannot be cleaned up.

What would be a better approach to this problem, so that if construction
fails for whatever reason, the system is left in a pristine state?

Thanks for your advice
Phil

Example SSCCE:
package exceptionInCtor;

import java.util.ArrayList;
import java.util.List;

public class Car {
private static List<String> nameList = new ArrayList<String>();
private String name;

public Car(String name){
nameList.add(name); // name is added to list as side-effect
this.name = name;
}

public void remove(){ // called when car is no longer used
nameList.remove(name);
}

public static class Ford extends Car{
public Ford(String name, int wheels){
super(name);
if(wheels > 4){
throw new IllegalArgumentException("Too many wheels");
}
}
}


public static void main(String[] args) {
Car c = null;
try{
c = new Ford("myFord", 5);
} catch (Exception e) {
// cannot call c.remove() here!
e.printStackTrace();
}

System.out.println(c); // prints null
System.out.println(nameList); // prints [myFord], should be empty
}
}

Since nameList is static, you could provide a static remove(String name)
method, and call that from the catch block.

Better though for constructors not to have side-effects. You could use
the Factory pattern and have the factory keep the name list.

H.
--
Hendrik Maryns
http://tcl.sfs.uni-tuebingen.de/~hendrik/
==================
http://aouw.org
Ask smart questions, get good answers:
http://www.catb.org/~esr/faqs/smart-questions.html


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4-svn0 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iD8DBQFIBLFUe+7xMGD3itQRArbdAJ9nH3d53jEcobOpbpn+uZLpK4yVggCfToV/
q23LkotqIJx9d2CZebvTw+8=
=6zk+
-----END PGP SIGNATURE-----

 

[Stefan Ram]

I have a class hierarchy where the constructor of a child class ("Ford")
can throw an exception, while the constructor of the parent class
("Car") has some side-effects.
If construction fails due to the Exception on the child class being
thrown, the side-effect of the parent constructor cannot be cleaned up.

What would be a better approach to this problem, so that if construction
fails for whatever reason, the system is left in a pristine state?

First, save all results of calls that might fail (throw)
into temporaries.

Then, perform the side effects with operations that cannot
fail (throw), for example, copy temporaries to permanent
variables.

This is called »exception-safe programming«.

»Widget& Widget:perator=( const Widget& other )
{
Widget temp( other ); // do all the work off to the side
Swap( temp ); // then "commit" the work using
return *this; // nonthrowing operations only
}«

http://www.gotw.ca/gotw/059.htm

See also:

http://www.gotw.ca/gotw/061.htm

 

[Jan Thomä]

What would be a better approach to this problem, so that if construction
fails for whatever reason, the system is left in a pristine state?

public class Car {
private static List<String> nameList = new ArrayList<String>();

Prolly not a good idea, as it is not threadsafe.

private String name;

public Car(String name){

this.name = name;
}

Create an initialize-method where you add the name to the list (register).
public void register() {
nameList.add( name );
}

public static class Ford extends Car{
public Ford(String name, int wheels){
super(name);
if(wheels > 4){
throw new IllegalArgumentException("Too many wheels");
}

else {
register();
}

}
}

Then you can first check the arguments and postpone the initialization
until you are pretty sure about that the arguments are correct. But of
course you can then forget to call register..

Otherwise you can call remove in the if-part

if(wheels > 4){ remove();
throw new IllegalArgumentException("Too many wheels");
}

Neither way is very elegant. A better way would be a car factory which does
the registration and also is thread safe:

class CarFactory {
private List<String> carNames;

public <T extends Car> create( Class<T> clazz ) {
T result = ... use reflection to create an instance...
register( result );
}

private synchronized void register(Car c) {
carNames.add(c.getName());
}
}

Jan

 

[Andreas Leitgeb]

Philipp schreef:
Where I learnt Java they told me two things about constructors:
1.) don't store away the "this" pointer! The object belongs to
who created it (with "new"), and only the owner shall
decide where to keep it, and who to pass it on.
2.) Don't throw exceptions from constructors.

ad 1:
Specifically with non-final classes, this can easily lead
to access of not-yet-completely initialized objects: Even
if the Ford-constructor did not throw an exception, other
threads might see the object (through the list) just before
Ford's c'tor has completed it's job.
ad 2:
I remember the rule, but not the exact reasons

Better though for constructors not to have side-effects.

Hmm, even stricter.

You could use the Factory pattern and have the factory
keep the name list.

That's of course the solution for such demands.

 

[Chase Preuninger]

Why would u need to clean up. U arnt using any system resources, the
garbage collector will do all cleanup for u. And any cleanup should
be done in the finally or catch block.

 

[Arne Vajhøj]

I have a class hierarchy where the constructor of a child class ("Ford")
can throw an exception, while the constructor of the parent class
("Car") has some side-effects.
If construction fails due to the Exception on the child class being
thrown, the side-effect of the parent constructor cannot be cleaned up.

What would be a better approach to this problem, so that if construction
fails for whatever reason, the system is left in a pristine state?

I would drop doing the stuff in constructors and do it in
ordinary methods. That will give you more freedom
to code exception handling as you want.

Arne

 

[Philipp]

I would drop doing the stuff in constructors and do it in
ordinary methods. That will give you more freedom
to code exception handling as you want.

Thanks to all for your answers. I learned a lot just by reading about
exception safety which was triggered by this thread.

In fact, I will follow the advice of several and add a register() method
which can handle the exception properly. Naturally, this puts the burden
of calling register() and remove() at the right moments on the programmer.

The two main other options, namely creating the instance using a factory
and using composition rather than inheritance represent a too large code
change for the expected result. In a future in-depth refactoring I will
definitely take these solutions into account.

Phil

 

[Philipp]

The two main other options, namely creating the instance using a factory
and using composition rather than inheritance represent a too large code
change for the expected result. In a future in-depth refactoring I will
definitely take these solutions into account.

PS: Could someone recommend some reference about why exceptions in
constructors are bad. In particular, if the constructor has no side
effects, I can't understand what the problem is in having exceptions.

Phil

 

[Stefan Ram]

PS: Could someone recommend some reference about why exceptions in
constructors are bad. In particular, if the constructor has no side
effects, I can't understand what the problem is in having exceptions.

For exception-safe programming, it helps when the constructor
does not has other side effects than the instance creation.
Otherwise, one needs to add a finally-clause to undo this.

I do not deem throwing constructors bad style. I believe that
exceptions even became popular in object oriented languages,
to support throwing constructors: A constructor cannot return
an error code. So exceptions are needed to communicate errors.

 

[Volker Borchert]

Philipp wrote:

|> PS: Could someone recommend some reference about why exceptions in
|> constructors are bad. In particular, if the constructor has no side
|> effects, I can't understand what the problem is in having exceptions.

One practical reason might be that, in upper layer methods of layered
architectures, you want to catch lower layer exception types thrown
from lower layer methods, and if you can't handle them, throw new
exceptions of upper layer exception types. But you are not allowed
to put the super() call inside a try...catch construct. So, callers
of your upper layer constructor will have to know about and deal with
lower layer exceptions.

 

[Volker Borchert]

Lew wrote:

|> I agree, throwing an exception from a constructor is perfectly legitimate if
|> properly documented. One problematic area is constructors that are invoked
|> reflectively, as via a framework or to load a driver. The framework must
|> expect the exceptions that can be thrown; if it assumes none can be and the
|> constructor throws one, it would be trouble.

Not really, since exceptions thrown from reflexive construction will
generally be wrapped into a (checked) InstantiationException.

 

[Daniel Pitts]

Hello

I have a class hierarchy where the constructor of a child class ("Ford")
can throw an exception, while the constructor of the parent class
("Car") has some side-effects.
If construction fails due to the Exception on the child class being
thrown, the side-effect of the parent constructor cannot be cleaned up.

What would be a better approach to this problem, so that if construction
fails for whatever reason, the system is left in a pristine state?

Thanks for your advice
Phil

Example SSCCE:
package exceptionInCtor;

import java.util.ArrayList;
import java.util.List;

public class Car {
private static List<String> nameList = new ArrayList<String>();
private String name;

public Car(String name){
nameList.add(name); // name is added to list as side-effect
this.name = name;
}

public void remove(){ // called when car is no longer used
nameList.remove(name);
}

public static class Ford extends Car{
public Ford(String name, int wheels){
super(name);
if(wheels > 4){
throw new IllegalArgumentException("Too many wheels");
}
}
}


public static void main(String[] args) {
Car c = null;
try{
c = new Ford("myFord", 5);
} catch (Exception e) {
// cannot call c.remove() here!
e.printStackTrace();
}

System.out.println(c); // prints null
System.out.println(nameList); // prints [myFord], should be empty
}
}

The answer is... Constructors should not cause side effects! Ever! For
just this reason.

The better approach is to use either a two-stage approach, or a factory
approach that encapsulates that two-stage approach

public abstract class Car {
final String name;
protected Car(String name) { this.name = name; }
public void init() { nameList.add(name); }

}
Car myCar = new Ford("Blah", 4);
myCar.init();

 

[Arne Vajhøj]

PS: Could someone recommend some reference about why exceptions in
constructors are bad. In particular, if the constructor has no side
effects, I can't understand what the problem is in having exceptions.

It is not always bad.

It can and should be used in some cases.

But sometimes it is an indication that functionality beyond
pure construction has been put into the constructor.

Arne

 

[Daniel Pitts]

It is not always bad.

It can and should be used in some cases.

But sometimes it is an indication that functionality beyond
pure construction has been put into the constructor.

Arne

Although, it may also indicate some validation problem during
construction.

A constructor is likely to throw NullPointerExceptions and
IllegalArgumentExceptions. They may also do some other validation and
throw some other exception.

I think that it may have been "bad" in non-gced langauges (such as C++),
where you could end up with memory leaks if the object wasn't properly
destructed.