0

This is a specific question here, but I'm interested in the general "best practice" for similar situations as I'm new to Java.

Suppose I have Java code that needs to open a file (see below for code). I first have a function that checks for the files existence. If the file exists, we call functions to open it and process it. Otherwise we return a message to the user stating the file could not be found.

Now in the functions that open the file, we still need to have a try/catch statement for the possible IOException because it's a checked exception. The function openSpecifiedFile has to return a FileInputStream. The fact that our file was proven to exist several milliseconds ago is not enough to guarantee the catch statement will never be executed (though it's unlikely) so I'd rather not return a null here.

Is there away to return a default object instead, or just avoid the null return statement all together and exit the program with some kind of runtime exception? The only way things could go bad here is if something very bad had happened I feel...

I suppose the general question is "When running checks to ensure certain checked exceptions shouldn't occur, what is a good way to deal with the necessary try/catch blocks?"

 public static void main(String[] args) {
    String filename = args[0];
    if (specifiedFileExists(filename)) {
        FileInputStream specifiedFile = openSpecifiedFile(filename);
        processFile(specifiedFile);
    } else
        System.out.println("The specified file does not exist");
}


private static boolean specifiedFileExists(String filename) {
    File currentFile = new File(filename);
    return currentFile.exists();
}

private static FileInputStream openSpecifiedFile(String filename) {
    try {
        return new FileInputStream(filename);
    } catch (IOException e) {}
    return null;
}

private static void processFile(FileInputStream currentFile) {
    ByteBuffer filledBuffer = fillBufferFromFile(currentFile);
    String messageFromFile = processBufferToString(filledBuffer);
    System.out.println(messageFromFile);
}

private static ByteBuffer fillBufferFromFile(FileInputStream currentFile) {
    try {
        FileChannel currentChannel = currentFile.getChannel();
        ByteBuffer textBuffer = ByteBuffer.allocate(1024);
        currentChannel.read(textBuffer);
        textBuffer.flip();
        return textBuffer;
    } catch (IOException e) {}
    return ByteBuffer.allocate(0);
}

private static String processBufferToString(ByteBuffer filledBuffer) {
    StringBuilder characterBuilderFromFile = new StringBuilder();
    while (filledBuffer.hasRemaining())
        characterBuilderFromFile.append((char) filledBuffer.get());
    return characterBuilderFromFile.toString();
}
  • A file being in use by another program or your user not having rights to open the file aren't that rare. – CodesInChaos Jan 14 '17 at 12:46
  • @CodesInChaos, that's a good point. I guess I'm more interested in the general case of how to deal with returning something when I'd rather just quit execution in this case – Johnathan Jan 14 '17 at 12:53
  • That's what exceptions are for. – Erik Eidt Jan 14 '17 at 16:24
0

While @gnasher729's suggestion is probably the best answer in this specific situation, there is a more general question here, which is (paraphrasing): "how do I handle checked exceptions that shouldn't ever happen?" And in cases where you cannot use them to eliminate a redundant (and potentially incorrect) check, the best alternative is to probably rethrow them as an unchecked exception. In most of my projects, I usually end up with an exception that looks like this:

 public class UnexpectedException extends RuntimeException
 {
       public UnexpectedException (Throwable cause, String whyItShouldntHaveHappened)
       { 
           super ("Caught an exception with description: " + cause.toString() + 
                  "\nThis shouldn't have happened because: " + whyItShouldntHaveHappened, 
                  cause); 
       }
 }

UnexpectedException is then handled at the top level of the application and is logged everywhere you could possibly think of logging it. Even if logging is turned off. Because any occasion where one actually gets thrown is direct evidence of a bug. It has no constructors other than the one shown, because you always need both parameters.

  • AssertionError already exists and more clearly captures the intent: some invariant was violated. – user22815 Jan 15 '17 at 19:16
  • @Snowman - Yes, I would recommend this except that many Java design guidelines state that subclasses of Error should never be caught, whereas catching and logging is the appropriate behaviour here. – Periata Breatta Jan 15 '17 at 19:19
  • You said it yourself: "guidelines" not "rules." I see no problem with a catch-and-rethrow of AssertionError. The reason why some people say not to catch errors is because there is typically not much that can be done to fix whatever caused them, or there is nothing that can be done after they are thrown (e.g. out of memory). – user22815 Jan 15 '17 at 19:23
  • @Periata Breatta, I think this is the kind of answer I was looking for. I constructed the above code as an example to highlight my general case but obviously it wasn't the best example. The method here seems good to me. I wanted to make my code not needing to have loads of exceptions in function signature to preserve the functions closedness. – Johnathan Jan 15 '17 at 20:52
7

Checking for the existence of a file and then opening it is always open to race conditions. Since opening the file will detect if the file doesn't exist and will handle it, checking for the existence of the file is actually pointless. Just open the file and handle exceptions.

I'd also say that you start with a filename, and from that filename you want to get a string. So write a function that takes a filename and returns a string or throws an exception. So one function that returns a string, given a filename, and one function that processes a string.

2

The main problem you face is that you really do not know what the program should do when the file cannot be opened inside the method openSpecifiedFile.

In a situation like this it is perhaps better to catch the exception and then rethrow it. If it makes sense to add further information to the exception then do that as well.

Returning a default object might very well surprise you or the users of your functions in the future.

When a file that existed a very short time earlier cannot be opened it is not a good practice to pretend nothing bad happened.

1

There are two problems here:

  1. I/O is prone to errors for a variety of reasons, which is why proper exception handling is important. There is no way to check that a file exists, and guarantee that it will continue to exist even a fraction of a second later. Other processes, including those buried in the operating system, may violate that assumption.

  2. What you propose is clunky and goes against what the vast majority of other programmers would assume how that code should be designed.

Thankfully, there is an easy fix.

Please take a look at the JavaDoc for IOException. At the top of the page is a list of subclasses. You can catch them, and in fact you should catch by subtype if you care about the program behaving differently based on the error condition.

try {
  FileInputStream in = getStream();
}
catch (FileNotFoundException e) {
  // ...
}
catch (IOException e) {
  // ...
}

This is how developers expect the code to be structured, because it is clear and works well. In those catch blocks you can wrap IOException in RuntimeException or even an Error. You could return null, or do anything else you want to do. But the error handling is consistent and located together.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.