Is there a clean way to give Java objects scope with destruction?

Question

Imagine that MyOpenedFile is something wrapping file with opened streams. Then suppose this code:

// method in an Util class
static void safeClose(MyOpenedFile f) {
  if (f != null) {
    try {
      f.close();
    } catch(IOException ex) { /* add logging or console output */ }
  }
}

Actual method for the question:

void doSomeFileOperation(...) throws IOException, ... {
    MyOpenedFile f1 = null;
    MyOpenedFile f2 = null;
    try {

      /* method's "business logic" code beings */
      f1 = new MyOpenedFile(...);
      //do stuff
      f2 = new MyOpenedFile(...);
      // do stuff
      f1.close(); f1 = null;
      // do stuff with f1 closed
      f2.close(); f2 = null;
      // do stuff with f2 closed
      /* method's "business logic" code ends */

    } finally {
      Util.safeClose(f1); f1 = null;
      Util.safeClose(f2); f2 = null; 
    }
}

Now this is quite messy and especially error-prone (some code in finally block might be very hard to get called in unit tests, for example). For example in C++, destructor would take care of cleanup (either getting called by scoped pointer destructor or directly) and code would be much cleaner.

So, is there better/nicer/cleaner way to wrap above piece of business logic code, so that any exceptions get propagated but both files f1 and f2 get closed (or at least close is attempted on both, even if it fails)?

Also answers pointing to any open source libraries such as Apache Commons, providing nice wrappers are welcome.

btw, your code sample has a mistake in it that will actually cause you to leak file handles because you set f1=null before you close it. — Chii, Nov 01 '12 at 10:15
@Chii No it doesn't, as far as I can see. There's `close` or `safeClose` *before* all `null` assignments. — hyde, Nov 01 '12 at 10:18
Why do you call `close` twice per file (in try and in finally)? — home, Nov 01 '12 at 10:19
@home I do not. I just want the dummy business logic describing a case which closes a file and sets reference to null. If reference is null in finally block, nothing is done to it. — hyde, Nov 01 '12 at 10:20
@hyde: ah i misread your sample. Which is interesting because its really easy to misread code written with this style of interspersed logic with housekeeping. But then, you cannot rewrite it because of the requirement that f1 be closed while f2 is kept open (otherwise, you could rewrite the sample in such a way that the logic is passed in, and the housekeeping squirreled away elsewhere — Chii, Nov 01 '12 at 10:27
I edited the example to use custom file class. Real code would probably use standard File and Stream classes, but this keeps the above code a bit cleaner I think. — hyde, Nov 01 '12 at 10:33
I don't follow why you'd specifically need to close your file(stream?)s before doing "stuff". If you need it to be written to file, you can just use `flush()`, it shouldn't be strictly necessary to close `f1` or `f2` before you're done with the method. Also, if that `MyOpenedFile` class is written by you, you could easily make it only call `close()` on the underlying stream the first time it's called, removing the need for a `safeClose` method. — Vala, Nov 01 '12 at 11:57
For example, there might be something which takes long time at the last part of business logic (for example making a HTTP request), and file is closed before that to release OS resources as early as possible. Sometimes it could be optimized, sometimes not, that's beside the point of cleaner overall solution (and in Java7 there is). — hyde, Nov 01 '12 at 12:05

score 4 · Accepted Answer · answered Nov 01 '12 at 10:28

4

A file is a wrapper for a String which contains a file name which may or may not exist. It is stateless so you don't need to close it.

A resource you need to close is FileInputStream or BufferedReader and you can close these implicitly with ARM in Java 7

try(BufferedReader br = new BufferedReader(new FileReader(file))) {

}

This will close br whent he block exits.

http://www.oracle.com/technetwork/articles/java/trywithresources-401775.html

answered Nov 01 '12 at 10:28

Peter Lawrey

525,659
79
751
1,130

1

I chose to accept your answer for being most to the point, and with IMO good code sample (sometimes small is beautiful), thank you. Now all that remains for me is, I need to find out if I can use Java7 for the actual thing... :-/ – hyde Nov 01 '12 at 10:46

maba · Answer 2 · 2012-11-01T10:36:16.923

1

Take a look at The try-with-resources Statement which will close resources after the try-block ends.

The File class you use does not seem to be java.io.File because it does not have any close() method. In that case make sure that your own File class implements Closeable to make it work with ARM.

try (FileInputStream f1 = new FileInputStream("test1.txt");
     FileInputStream f2 = new FileInputStream("test2.txt")) {
    // Some code
} catch (FileNotFoundException e) {
    e.printStackTrace();
} catch (IOException e) {
    e.printStackTrace();
}

edited Nov 01 '12 at 10:36

answered Nov 01 '12 at 10:27

maba

47,113
10
108
118

Thanks pointing out the `File` thing, I changed class name and added comment to question explaining the logic behind custom class name. – hyde Nov 01 '12 at 10:34
1

One nitpick, more like general ranting not directed at you: I think that kind of catch blocks should be used very rarely. It is almost always bad idea to just catch exception, log it, and continue as if nothing happened. If exception can be safely ignored, logging it is just clutter. If action needs to be taken, then it should be more than just logging it (like, pop up a dialog in GUI app). Mostly exception should be allowed to propagate up, sometimes as a cause of another more relevant exception. Just logging is useful mostly when *programmer* is unsure of the right thing to do. – hyde Nov 01 '12 at 10:53

score 1 · Answer 3 · edited May 23 '17 at 12:15

You dont need to close Files (which are representations of files on the file system) as mentioned here:

Do I need to close files I perform File.getName() on?

I assume you are asking more about the File Streams/Readers?

In which case java 7 has a nice new feature: http://www.vineetmanohar.com/2011/03/java-7-try-with-auto-closable-resources/

If you are working on an older version of java I'd just keep it simple with this:

void doSomeFileOperation(...) throws IOException, ... {
  FileInputStream f1 = null;
  FileInputStream f2 = null;
  try {

    // do stuff

  } finally {
    Util.safeClose(f1); 
    Util.safeClose(f2); 
  }
}

score 0 · Answer 4 · answered Nov 01 '12 at 10:25

An option that comes automatically to my head is: Separate the code that handles files, from the code that does any processing. In this way you can encapsulate the nasty code that handles does the open, close and exception handling.

The other bit is that the sample you have does a lot of extra, unneeded steps

void doSomeFileOperation(...) throws IOException, ... {
    File f1 = null;
    File f2 = null;
    try {
      f1 = new File(...);
      f2 = new File(...);
      // callback to another class / method that does the real work
    } finally {
      Util.safeClose(f1);
      Util.safeClose(f2);
    }
}

You don't need to set the File instances to null. If you try to use them, you'll get an exception.

I don wonder what File object you're using. The standard File class in java doesn't have a close() method.

i initially thought the same, but the flaw is that the requirement that `// do stuff with f1 closed` is no longer kept. You'd need to pass in two different callbacks, and so this didn't really fix the problem. — Chii, Nov 01 '12 at 10:29
Yes, depending on exact business code, the cleanup could be optimized. But this leads to *different* boilerplate code for different cases, which might be even worse. — hyde, Nov 01 '12 at 10:36

Is there a clean way to give Java objects scope with destruction?

4 Answers4