views:

560

answers:

7

"The major difference between a thing that might go wrong and a thing that cannot possibly go wrong is that when a thing that cannot possibly go wrong goes wrong it usually turns out to be impossible to get at or repair." -Douglas Adams

I have an class FileItems. FileItems constructor takes a file, and throws an exception (FileNotFoundException) if the file doesn't exist. Other methods of that class also involve file operations and thus have the ability throw the FileNotFoundException. I would like to find a better solution. A solution which doesn't require that other programmers handle all of these extremely unlikely FileNotFoundExceptions.

The facts of the matter:

  1. The file has been checked to exist but the extremely unlikely possibility exists that through some major fault of reality the file might be deleted before this method is called.
  2. Since the probability of 1 happening is extremely unlike and unrecoverable, I would prefer to define an unchecked exception.
  3. The file is has already been found to exist, forcing other programmers to write code and catch the checked FileNotFoundException seems tedious and useless. The program should just fail completely at that point. For example there is always the chance that a computer may catch fire, but no one is insane enough to force other programmers to handle that as a checked exception.
  4. I run into this sort of Exception issue from time to time, and defining custom unchecked exceptions each time I encounter this problem (my old solution) is tiresome and adds to code-bloat.

The code currently looks like this

 public Iterator getFileItemsIterator() {
    try{
        Scanner sc = new Scanner(this.fileWhichIsKnowToExist);
        return new specialFileItemsIterator(sc);     
       } catch (FileNotFoundException e){ //can never happen} 

    return null;
 }

How can I do this better, without defining a custom unchecked FileNotFoundException? Is there some way to cast a checkedException to an uncheckException?

+8  A: 

You can convert a checked exception to an unchecked by nestling it inside a RuntimException. If the exception is caught higher up in the stack and outputted using printStackTrace(), the stack for the original exception will be displayed as well.

try {
    // code...
}
catch (IOException e) {
    throw new RuntimeException(e);
}

This is a good solution, that you shouldn't hesitate to use in these situations.

Emil H
+21  A: 

The usual pattern to deal with this is exception chaining. You just wrap the FileNotFoundException in a RuntimeException:

catch(FileNotFoundException e) {
    throw new RuntimeException(e);
}

This pattern is not only applicable when an Exception cannot occur in the specific situation (such as yours), but also when you have no means or intention to really handle the exception (such as a database link failure).

Edit: Beware of this similar-looking anti-pattern, which I have seen in the wild far too often:

catch(FileNotFoundException e) {
    throw new RuntimeException(e.getMessage());
}

By doing this, you throw away all the important information in the original stacktrace, which will often make problems difficult to track down.

Another edit: As Thorbjørn Ravn Andersen correctly points out in his response, it doesn't hurt to state why you're chaining the exception, either in a comment or, even better, as the exception message:

catch(FileNotFoundException e) {
    throw new RuntimeException(
        "This should never happen, I know this file exists", e);
}
Henning
+1 on the anti-pattern. that drives me nuts.
akf
+1, I didn't think RuntimeException constructor took an exception due to all the code I've since that just passes the message. Thanks so much.
e5
Now go and visit dailywtf.com for a screenshot of a dialog which says "This should never happen, I know this file exists"
oxbow_lakes
When editing content inspired by other answers, at least have the curtesy to say that ;-)
Thorbjørn Ravn Andersen
@Thorbjørn: Sorry, I had not seen that you posted something along the same line already. Attribution added.
Henning
I think the antipattern is a holdover from long ago (1.3?) when Exception chaining did not exist.
Michael Borgwardt
+6  A: 

If your position is that this is so unlikely and should just end the program, use an existing runtime exception, even RuntimeException itself (if not IllegalStateException).

try {
  ....

} catch (FileNotFoundException e) {
    throw new RuntimeException(e);
}
Yishai
+1  A: 

You are probably better off throwing the superclass and more generic exception IOException at any point in your code which involves reading or writing from the file.

The file may exist when your class's constructor runs, but that doesn't guarantee that:

  1. It exists when methods are called
  2. It's writable/readable
  3. Another thread doesn't access it and somehow screw up your streams
  4. The resource doesn't go away in the middle of processing

etc.

Instead of reinventing the wheel, I would say just re-throw IOException wherever the JDK/java.io classes you are using force you to do so.

Also I for one hate classes that throw Exceptions from their constructor - I'd get rid of these if I were you.

matt b
+1  A: 

I did a little googling and found this glob of code. It's a bit more flexible of an approach me thinks

Compliments of this article

class SomeOtherException extends Exception {}

public class TurnOffChecking {
  private static Test monitor = new Test();
  public static void main(String[] args) {
    WrapCheckedException wce = new WrapCheckedException();
    // You can call f() without a try block, and let
    // RuntimeExceptions go out of the method:
    wce.throwRuntimeException(3);
    // Or you can choose to catch exceptions:
    for(int i = 0; i < 4; i++)
      try {
        if(i < 3)
          wce.throwRuntimeException(i);
        else
          throw new SomeOtherException();
      } catch(SomeOtherException e) {
          System.out.println("SomeOtherException: " + e);
      } catch(RuntimeException re) {
        try {
          throw re.getCause();
        } catch(FileNotFoundException e) {
          System.out.println(
            "FileNotFoundException: " + e);
        } catch(IOException e) {
          System.out.println("IOException: " + e);
        } catch(Throwable e) {
          System.out.println("Throwable: " + e);
        }
      }
    monitor.expect(new String[] {
      "FileNotFoundException: " +
      "java.io.FileNotFoundException",
      "IOException: java.io.IOException",
      "Throwable: java.lang.RuntimeException: Where am I?",
      "SomeOtherException: SomeOtherException"
    });
  }
} ///:~
mugafuga
+5  A: 

Consider using the form

throw new RuntimeException("This should never happen", e);

instead. This allows you to convey meaning to the maintainer to follow you, both when reading the code but also SHOULD the exception happen to be thrown in some strange scenario.

EDIT: This is also a good way to pass exceptions through a mechanism not expecting those exceptions. E.g. if you have a "get more rows from the database" iterator the Iterator interface does not allow to throw e.g. an FileNotFoundException so you can wrap it like this. In the code USING the iterator, you can then catch the runtimeexception and inspect the original excpetion with getCause(). VERY useful when going through legacy code paths.

Thorbjørn Ravn Andersen
+1  A: 

I have experienced the same problem.

In the simplest case you inform the user of the error and suggest to either to repeat or to cancel the operation.

In general case, workflow is a sequence of actions (I/O including), where each action "assumes" that the previous one had succeeded.

The approach I have chosen is to create a list of 'roll-back' actions. If the workflow succeeds, they are ignored. If an exception occurs, I execute rollbacks and present an exception to the user.

This:

  • keeps integrity of data
  • allows to code much easier

Typical function is like:

returntype func(blah-blah-blah, Transaction tr)
{
    // IO example
    Stream s = null;
    try
    {
        s = new FileStream(filename);
        tr.AddRollback(File.Delete(filename));
    }
    finally
    {
        if (s != null)
            s.close();
    }
}

Typical usage is:

Transaction tr = new Transaction();
try
{
    DoAction1(blah1, tr);
    DoAction2(blah2, tr);
    //...
}
catch (Exception ex)
{
    tr.ExecuteRollbacks();
    // queue the exception message to the user along with a command to repeat all the actions above
}

This is a little bit trickier in real-world, because

  • sometimes it is necessary to obtain a bunch of locks before execution actions
  • rollback code should be exception-silent itself (for instance)

But I have already get used to this approach, now my applications are more stable.

modosansreves