Clean up after exception thrown in constructor

Discussion in 'Java' started by Philipp, Apr 15, 2008.

  1. Philipp

    Philipp Guest

    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
    }
    }
     
    Philipp, Apr 15, 2008
    #1
    1. Advertising

  2. 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-----
     
    Hendrik Maryns, Apr 15, 2008
    #2
    1. Advertising

  3. Philipp

    Stefan Ram Guest

    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?


    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::eek: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
     
    Stefan Ram, Apr 15, 2008
    #3
  4. Philipp

    Jan Thomä Guest

    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
     
    Jan Thomä, Apr 15, 2008
    #4
  5. > 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 <> 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.
     
    Andreas Leitgeb, Apr 15, 2008
    #5
  6. 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.
     
    Chase Preuninger, Apr 16, 2008
    #6
  7. Philipp

    Arne Vajhøj Guest

    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
     
    Arne Vajhøj, Apr 16, 2008
    #7
  8. Philipp

    Philipp Guest

    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, Apr 16, 2008
    #8
  9. Philipp

    Philipp Guest

    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
     
    Philipp, Apr 16, 2008
    #9
  10. Philipp

    Stefan Ram Guest

    Philipp <> 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.
     
    Stefan Ram, Apr 16, 2008
    #10
  11. 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 <>
    "I'm a mechanic, not a doctor." Volker Borchert <>
     
    Volker Borchert, Apr 19, 2008
    #11
  12. 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 <>
    "I'm a mechanic, not a doctor." Volker Borchert <>
     
    Volker Borchert, Apr 19, 2008
    #12
  13. Philipp

    Daniel Pitts Guest

    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/>
     
    Daniel Pitts, Apr 20, 2008
    #13
  14. Philipp

    Arne Vajhøj Guest

    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, Apr 26, 2008
    #14
  15. Philipp

    Daniel Pitts Guest

    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/>
     
    Daniel Pitts, Apr 26, 2008
    #15
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Selen
    Replies:
    0
    Views:
    2,688
    Selen
    May 28, 2004
  2. =?Utf-8?B?VmFs?=
    Replies:
    0
    Views:
    3,140
    =?Utf-8?B?VmFs?=
    Jun 8, 2005
  3. Replies:
    1
    Views:
    625
    Ivan Vecerina
    May 15, 2006
  4. jobs
    Replies:
    1
    Views:
    1,896
    Scott Roberts
    Nov 16, 2007
  5. Val
    Replies:
    0
    Views:
    185
Loading...

Share This Page