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
   }
}

This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
--------------enigA7899F9A7548A70E301EAE1B
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable

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

>=20
> What would be a better approach to this problem, so that if constructio=
n=20
> fails for whatever reason, the system is left in a pristine state?
>=20
> Thanks for your advice
> Phil
>=20
> Example SSCCE:
> package exceptionInCtor;
>=20
> import java.util.ArrayList;
> import java.util.List;
>=20
> public class Car {
>   private static List<String> nameList =3D new ArrayList<String>();
>   private String name;
>=20
>   public Car(String name){
>     nameList.add(name); // name is added to list as side-effect
>     this.name =3D name;
>   }
>=20
>   public void remove(){ // called when car is no longer used
>     nameList.remove(name);
>   }
>=20
>   public static class Ford extends Car{
>     public Ford(String name, int wheels){
>       super(name);
>       if(wheels > 4){
>         throw new IllegalArgumentException("Too many wheels");
>       }
>     }
>   }
>=20
>=20
>   public static void main(String[] args) {
>     Car c =3D null;
>     try{
>       c =3D new Ford("myFord", 5);
>     } catch (Exception e) {
>       // cannot call c.remove() here!
>       e.printStackTrace();
>     }
>=20
>     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=20
the Factory pattern and have the factory keep the name list.

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


--------------enigA7899F9A7548A70E301EAE1B
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----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-----

--------------enigA7899F9A7548A70E301EAE1B--

Philipp <sicsicsic@freesurf.ch> writes:
>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::operator=( 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
 

On Tue, 15 Apr 2008 15:38:38 +0200  Philipp wrote:
> 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

Philipp writes:
>> 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?

Stefan Ram wrote:
>   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::operator=( 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
>       }«

My Java compiler doesn't seem to handle this example very well.

Of course, there is an equivalent idiom in Java, but I'm not sure it handles 
the OP's issue.  The parent class cannot know that the child class's part of 
the constructor will demand an unwind of the setup actions.  The child class 
will need appropriate try-catch semantics:

  public Child()
  {
   try
   {
    somethingThatThrows()
   }
   catch( Exception exc )
   {
     unwindParentClassActions();
     throw exc;
   }
  }

Since the parent constructor cannot unwind the actions, obviously the child 
constructor has to.

One might want to move the child method that could throw an exception out of 
the constructor.  Also the parent class actions that create side effects could 
move out of that constructor.  It's a tough call.  Constructors ideally should 
only do what's necessary for construction of an object in memory.  This makes 
one push side effects out into methods.  However, RAII ("Resource Acquisition 
is Initialization") argues that a resource should attach to the lifetime of 
its respective resource manager, which in turn leads one to put the resource 
acquisition in the constructor.  But then one runs into cases like the current 
matter, where some client of the code messes with the assumptions.

Perhaps the child class shouldn't really inherit from the parent.  Perhaps it 
should compose an instance of the resource manager instead of deriving from it.

-- 
Lew

> Philipp schreef:
>> 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.

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 :-)

Hendrik Maryns <gtw37bn02@sneakemail.com> wrote:
> 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.

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.

Philipp wrote:
> 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

Chase Preuninger wrote:
> 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.

Wow.  Why is it too much work to type the "yo" part of the word "you"?  Just 
spelling it "u" looks singularly jejune.

GC does not do "all cleanup for u [sic]".  It releases memory.  That's all. 
The OP spoke of side effects.  They didn't say these effects were limited to 
memory; in context one would reasonably conclude that they were not.  Ergo, 
cleanup is very definitely a concern.

-- 
Lew

Arne Vajh�j wrote:
> Philipp wrote:
>> 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.

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 wrote:
> 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

On Tue, 15 Apr 2008 23:30:35 -0700, Philipp <sicsicsic@freesurf.ch> 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.

I can't either.  :)

Seriously though, lots of classes in the Java SDK can throw exceptions  
 from the constructor.  I didn't read any of the messages in this thread as  
being anti-exception in a constructor.  Just anti-side-effects.  And of  
course it seems as though you _do_ understand at least some of why  
side-effects in a constructor are bad (frankly, they're not great  
anywhere, but it seems to me they are especially non-optimal in a  
constructor).

I suppose if someone was specifically arguing against constructors  
throwing exceptions, they will respond explaining why.  If that happened  
in this thread, I missed it.

I don't personally think that there's anything fundamentally wrong with  
throwing an exception from a constructor.  After all, every "new" can  
potentially throw at least one exception (out-of-memory).  And throwing an  
exception from a constructor is the only way to report an error (be it an  
invalid parameter, some problem with the current state of the program,  
etc.)

Pete

Philipp <sicsicsic@freesurf.ch> writes:
>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.

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.

Peter Duniho wrote:
> I can't either.  :)
> 
> Seriously though, lots of classes in the Java SDK can throw exceptions 
> from the constructor.  I didn't read any of the messages in this thread 
> as being anti-exception in a constructor.  Just anti-side-effects.  And 
> of course it seems as though you _do_ understand at least some of why 
> side-effects in a constructor are bad (frankly, they're not great 
> anywhere, but it seems to me they are especially non-optimal in a 
> constructor).
> ... 
> I don't personally think that there's anything fundamentally wrong with 
> throwing an exception from a constructor.  After all, every "new" can 
> potentially throw at least one exception (out-of-memory).  And throwing 
> an exception from a constructor is the only way to report an error (be 
> it an invalid parameter, some problem with the current state of the 
> program, etc.)

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.

Philipp wrote:
>> 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.

You can assist the client code with checks for proper state in the service 
methods.  If register() has not been called yet, then doWork() can throw an 
IllegalStateException.  Calling on an improperly initialized object is a 
programming error, so a runtime exception is appropriate.

Even though runtime exceptions are not declared in the method signature, you 
should still document them with an '@throws' in the Javadoc.

-- 
Lew

Lew wrote:
> Chase Preuninger wrote:
>> 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.

> GC does not do "all cleanup for u [sic]".  It releases memory.  That's 
> all. The OP spoke of side effects.  They didn't say these effects were 
> limited to memory; in context one would reasonably conclude that they 
> were not.  Ergo, cleanup is very definitely a concern.

Also, in this specific example, the garbage collector won't be able to
collect everything.  There is a String that has been added to a static
list of Strings.  The garbage collector can't collect that because it
is still live data.

   - Logan

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.

-- 

"I'm a doctor, not a mechanic." Dr Leonard McCoy <mccoy@ncc1701.starfleet.fed>
"I'm a mechanic, not a doctor." Volker Borchert  <v_borchert@despammed.com>

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.

-- 

"I'm a doctor, not a mechanic." Dr Leonard McCoy <mccoy@ncc1701.starfleet.fed>
"I'm a mechanic, not a doctor." Volker Borchert  <v_borchert@despammed.com>

Philipp wrote:
> 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();

-- 
Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>

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.

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

Arne Vajh�j wrote:
> 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.
> 
> 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.

-- 
Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>