CODEWITHSUNDEEP
java
TownCube
TownCube

Reputation: 1310

Cleaning up after exceptions thrown during contruction

I'm currently instansating a number of objects within my constructor that require the .close() be called on them when you're done. If in that sequence one object throws an exception, how do I clean up the objects already allocated so far. This presents an issue where the calling function (even if it used try-finally) would never get a reference to call the .close as the object never finished construction.

Thoughts I've had:

Example code:

class MyClass implements AutoCloseable {
  private EarthConnection earth;
  private SolarConnection solar;
     public MyClass() {
        earth = new EarthConnection();
        solar = new SolarConnection(); // exception thrown by this connection
     }

     public close() {
        if (earth != null) { 
            earth.close();
        }
        if (solar != null) {
            solar.close();
        }
    }
}

   // Caller
   try (MyClass myClass = new MyClass()) {
       // do work - note if MyClass wasn't fully constructored it can't call the close method on it.
   }

In the example above how do I clean up the allocated EarthConnection if the SolarConnection throws an exception?

Upvotes: 2

Views: 667

Answers (5)

fgb
fgb

Reputation: 18569

Catching Exception and rethrowing should do it. This allows the callers to see the original exception, but allows the cleanup required in the class to close resources.

public MyClass() {
    try {
        earth = new EarthConnection();
        solar = new SolarConnection();
    } catch(Exception e) {
        close();
        throw e;
    }
}

Upvotes: 0

NoDataFound
NoDataFound

Reputation: 11959

I would personally do this: instead of constructor, a static method to wrap the ugly code leaving your constructor simple;

In your case, you don't have to catch all exception, only those that come after the first initialization : solar is created after earth (that's probably not the case in the universe :)), so only if it fails, you'll need to clean earth the hard way.

You must not use try-with-resources here, because you would close your resource which is not what you want.

class MyClass implements AutoCloseable {
  private EarthConnection earth;
  private SolarConnection solar;
     private MyClass(EarthConnection earth, SolarConnection solar) {
        earth = new EarthConnection();
        solar = new SolarConnection(); // exception thrown by this connection
     }

     public static MyClass newMyClass() {
       EarthConnection earth = new EarthConnection();
       try {
         SolarConnection solar = new SolarConnection(); 
         return new MyClass(earth, solar);
       } catch (SolarException e) {
         earth.close(); // may throw, you can ignore it.
         throw e;
       }       
     }
...
}

And if you have more than earth and solar, for example mars and venus, you would probably need to use a class wrapping List of AutoCloseable, however your need to close object in reverse order of registering (pretty much like how C++ destructor are called in reverse order of construction).

class MyClass implements AutoCloseable {
  private final EarthConnection earth;
  private final VenusConnection venus;
  private final MarsConnection mars;
  private final SolarConnection solar;
  private final AutoCloseable cl;

  private MyClass(
    final EarthConnection earth,
    final VenusConnection venus,
    final MarsConnection mars,
    final SolarConnection solar,
    final AutoCloseable ac
  ) {
    this.earth = earth;
    this.venus = venus;
    this.mars = mars;
    this.solar = solar;
    this.cl = cl;
  }

public static MyClass newMyClass() {
  AutoCloseables cl = new AutoCloseables<>();
  try {
    EarthConnection earth = cl.register(new EarthConnection());
    VenusConnection venus = cl.register(new VenusConnection ());
    MarsConnection  mars = cl.register(new MarsConnection());
    SolarConnection solar = cl.register(new SolarConnection());

    return new MyClass(earth, venus, mars, solar, cl);
  } catch (EarthException | VenusException | MarsException | SolarException e) {
    cl.close();
    throw e; // or new MyClassException(e);
  }
}

@Override
public void close() {
  cl.close();
}

With:

class AutoCloseables {
  private final List<AutoCloseable> list;
  public <E extends AutoCloseable> E register(E ac) {list.add(ac); return ac;}
  @Override
  public void close() {
    Collections.reverse(list); // destroy in reverse order
    for (AutoCloseable ac : list) {
      try {ac.close();}
      catch (Exception e) {
        // IGNORED or you may use supressedException https://docs.oracle.com/javase/7/docs/api/java/lang/Throwable.html#addSuppressed(java.lang.Throwable)
      } 
    }
  }
}

Upvotes: 0

user2575725
user2575725

Reputation:

Keep it simple, a class should be responsible for its own state instead of MyClass doing job to close resources for other classes. This will also ensure less code changes to MyClass with additional new class in future.

IMO, you may try this out:

class EarthConnection implements AutoCloseable {
    @Override
    public void close(){
        /* TO DO */
    }
}

class SolarConnection implements AutoCloseable {
    @Override
    public void close(){
        /* TO DO */
    }
}

class MyClass {
    private EarthConnection earth;
    private SolarConnection solar;
    public MyClass(EarthConnection earth, SolarConnection solar) {
        this.earth = earth;
        this.solar = solar;
    }
    /* TO DO */
}

try(EarthConnection earth = new EarthConnection()){
    try(SolarConnection solar = new SolarConnection()){ /* exception thrown by this connection*/
        MyClass myClass = new MyClass(earth,solar);
        /* TO DO */
    }
}

Upvotes: 0

Oleg Cherednik
Oleg Cherednik

Reputation: 18245

It is bad practice to rely on catching exception from constructor. I would recommend to use factory method an catch exception there: try...catch with recources

Upvotes: 0

Jakub Dyszkiewicz
Jakub Dyszkiewicz

Reputation: 534

How about try-with-resources (assuming you are using Java 7 or later)? If you create object in try section then it will be closed automatically without catching exceptions. The only disadvantage of this approach is that you can still create object outside try section and then it won't be closed.

Upvotes: 1

Related Questions