views:

360

answers:

6

What is the best practice to follow when you need to throw an exception which was not defined in an interface that you are implementing?

Here is an example:

public interface Reader
{
    public abstract void read() throws IOException;
}

public class CarrotReader implements Reader
{
    public void read() throws IOException {}
}

public class CupcakeReader implements Reader
{
    public void read() throws IOException, CupcakeException {}
}

In this case, you have a specific exception that occurs when reading cupcakes, so you want to throw an exception related to this. However, Reader doesn't define this type of exception in its interface, so what do you do? Furthermore, it doesn't make sense to add CupcakeException to the throws clause in the Reader interface, because this type of exception is specific to CupcakeReader. One way around this is to have Reader define read such that it throws some parent type, like Exception, but then you lose the context for the exception. What should you do in this situation? Thanks!


Another interesting situation that has been brought up involves an interface over which you have no control. In this case, what is the best way to indicate that a problem has occurred?

For illustrative purposes, here is another example:

public interface Reader
{
    public abstract void read();
}

public class CupcakeReader implements Reader
{
    public void read() throws CupcakeException {}
}

In this case, you cannot change Reader, but you want to indicate that a problem has occurred in CupcakeReader's read method.

A: 

Perhaps you could make an abstract ReaderSpecificException class, put it in the Interface, and subclass CupcakeException from this abstract class.

Zed
I don't think it is a good practice to create an exception as an abstract class. By default, an exception without any attributes in it can still provide insight into the problem (just by choosing right name).Why would you want to define it as an abstract class?
Manglu
+7  A: 

Use something called ReaderException that will serve as the root interface of your exception hierarchy. ReaderException will also provides a link to other exceptions that get thrown due to lower level exceptoins.

CodeToGlory
Isn't this the same thing as adding Exception to the throws clause? I realize that by defining your own parent class that is somewhat more specific, you preserve more of the context, but at some level you're still losing information, right?
Scott
That's what I usually do (+1). But what would you do if you could not change the interface (the code's not yours, the code's been already shipped, etc)?
Martinho Fernandes
@Scott: you don't lose anything. If you throw a `CupcakeException` that extends `ReaderException`, you can catch it either as a `ReaderException` or as a `CupcakeException`, depending on how specific you want to be.
Martinho Fernandes
So, in your catch block, you would have to catch the parent type (e.g. Exception) and then check using instanceof to find out what it really is, right? This seems kind of messy. I also would like to know the answer to you question regarding not having control over the interface.
Scott
If you use exception chaining as per Thorbjørn's answer, I think that's the only way to go. If you can change the interface, simply use two catch blocks, one for Cupcake, another for general Reader, and make sure the Cupcake block comes first.
Martinho Fernandes
This is not the same as cathching Exception. In fact, i woudl call catching Exception as a sin because you end up catching other runtime exceptions unintentionally. As an example, the catch block would also receive NPE if one were to occur. The calling code would not be interested in catching and handling these. So using your own Exception hierarchy is the right way to go. The client gets the ability to pick and choose to catch the right exception and do the appropraite handling (if he so desires to)
Manglu
+6  A: 

You may have to create an exception of the expected type instead.

... catch(CupcakeException e) {
   throw new IOException("The sky is falling", e);
 }
Thorbjørn Ravn Andersen
+1 for mentioning exception chaining.
Martinho Fernandes
This is what I'm doing at the moment.
Scott
This is probably the best solution if you can't change the interface. You can still access all the information you might need, and you still follow the interface's contract.
Martinho Fernandes
@Scott, then the calling code can catch the IOException, and if a cause is present, then use THAT instead. You cannot pressure more checked exceptions through an interface without changing it. An interface is a _contract_.
Thorbjørn Ravn Andersen
A: 

If you create a higher abstract exception that works as a base class for CupCakeException you don't bind the Reader Interface to a specific implementation like you would be doing if you added the CupCakeException to the Reader interface. If you don't let one Exception inherit from another there is a constructor in the exception class that takes a throwable as second argument like Thorbjørn Ravn Andersen allready showed in his short code example. The enables you to generate a more abstract exception and every part of your code that needs to know more then just "there is an error" can look for the cause of the higher exception.

Janusz
A: 

Just don't use checked exceptions.

The example you showed is one of the reasons checked exceptions are bad.

The main reason though is that the user of your cupcake reader will have to handle your exception regardless of whether he is interested in it or not.

So instead of:

Value value = reader.read();

You are forcing him to do this:

Value value = null;
try {
    value = reader.read();
} catch (Exception e) {
    // now what??
}

value.doSomething();   // potential NPE here

Think which one is better, more readable and less error prone and just stop using checked exceptions.

EDIT:

I am surprised with the negative rating. Are there people who still think that checked exceptions are great? If so here are some references why you shouldn't use checked exceptions:

  • No modern framework uses checked exceptions (Spring, EJB3 etc)
  • Article with code examples here
  • StackOverflow topic
  • Effective Java (sections 58 and 59) - here
Gregory Mostizky
I think you're posting a deliberately misleading example. I would write the above to declare and use the variable within the try{} block. There's no reason you should have to write code like the above (regardless of whether exceptions are checked or not)
Brian Agnew
The problem with checked exceptions is not the one presented here. As Brian pointed out, this code is just wrong. One problem with checked exceptions is that you lose any benefit of doing pre-emptive checks (when you can) before calling code that throws exceptions. Even if you do pre-emptive checks, you're forced to use a try-catch block. Another is the fact that if you have something N method calls deeps that throws, and only call 0 can handle it, every call from 0 to N has to include throws in the contract (or cheat with `RuntimeException`s
Martinho Fernandes
Read this for more info on the "why-not"s of checked exceptions: http://www.artima.com/intv/handcuffs.html
Martinho Fernandes
I don't believe you can realistically do pre-emptive checks. Who's to say that the conditions remain the same between when you make your check and when you execute the dependent code. I realise the two will happen very closely together timewise, but that's still no guarantee.
Brian Agnew
@Brian: Sure you can. I am about to load a config file, so I check that it's there. If it is, I continue with loading, no need for try/catch. If some ID10T sys admin is sitting there deleting my files from under me, that's a legitimate reason to blow up in a screaming heap with an unchecked exception - since it will never happen in *practice* and if it does, we have bigger problems than my missing config file (namely the sys admin who deletes things willy nilly and without authorization).
Software Monkey
@Brian: I've seen quite a lot of such examples in real code. Sometimes it's possible to put the value into the try..catch and sometimes it's even more messier if you do - think of multiple statements each throwing different exception and what happens if you try to put them in a single try..catch
Gregory Mostizky
+1  A: 

Exception is part of the interface. Define a generic parent for all your exceptions in the interface if you can redefine the interface.

You can also make CupcakeException a child of IOException.

ZZ Coder
Have a "up vote" for the "also". But I am biased, because that is what I would most likely have done (subclass of the declared exception, if needed). Of course, having a subclass of (e.g.) IOException assumes the caller needs to actually care that something more specific happened, as well as implying that the problem really is some kind of I/O related problem.
Roboprog