Do you clean before you make mess? On putting cleanup code inside finally block

I have a question concerning exception handling and resource management and I was wondering if anybody could share their opinion. I need to perform a sequence of actions: read app settings, setup the environment, do the stuff and then eventually clean up. Cleaning up involves tearing down the environment, but this should only happen if it was successfully setup in the first place.

Here's my first (and lame) approach:

try {
 readSettings();
 setupEnvironment();
} catch (Exception ex) {
 logStackTrace(ex);
 displayError(ex);
 closeCommThreads();
 return;
}

try {
 // do stuff
} catch (Exception ex) {
 logStackTrace(ex);
 displayError(ex);
} finally {
 teardownEnvironment();
 closeCommThreads();
}

That seemed a bit ugly, so I decided to look for a better solution. I did some background reading and quite many articles vote for bigger try/catch blocks and using (a pun?) finally for cleanup. So here's my second attempt:

try {
 readSettings();
 setupEnvironment();
 // do stuff
} catch (Exception ex) {
 logStackTrace(ex);
 displayError(ex);
} finally {
 teardown开发者_开发百科Environment();
 closeCommThreads();
}

To make this work I had to remove sequential coupling from teardownEnvironment() so that it can be invoked anytime - before or after setupEnvironment() (for editors: any way of putting it better?). Is this the right approach? I does feel slightly weird to tear down before setting up.

Edit:

Just to make it a bit more explicit: I removed sequential coupling by including an extra check inside teardownEnvironment - something like if (!isSetup()) return;.

---

Neither is right or wrong if both ways accomplish what you intend to do.

In my opinion the second approach (big try/catch/finally block) is easier to read because you don't have a return statement "hidden" in a catch block somewhere in your method body.

Moreover, I think removing the interdependencies between methods (teardown, setup) is generally a good practice (e.g. you can unit test these methods independently). Just exit from teardown if nothing has been setup - no need to feel weird about it :)

---

The finally approach is used to have a guarantee that before an exception is thrown, the resources used can be closed/dereferenced (and ready for garbage collection). This is to just to promote exception safety.

What you've just discussed here is called the Dispose Pattern. Basically, because of the automatic nature of the Java GC, you're doing resource cleanup for the runtime environment.

Your second approach is correct (which is a Dispose Pattern), as you're effectively cleaning your environment with tearDownEnvironment(), closing all resources before the JVM GC kicks in.

Extra info: Resource Acquisition Is Initialization could enlighten you?

---

Well typically your teardownEnvironment() would check for

isEnvironmentSetCorrectly() or if(environment != null) or whatever similar

before you start tearing down.

So you wont have to "feel slightly weird" about it, since it wont happen if done correctly.