How to tell caller of an error if you cant change the signature of amethod

[Royan]

Lets say you have the following interface:

Interface IFoo {
public String[] readStrings();
}



The thing is that you cannot change that interface, but you have to
implement it. Now that readStrings() method must read from file, then
do some parsing and finally retrieve an array of Strings. As you can
imagine a couple of exceptional conditions have to be handled within
the implementation of this method:

1) IOException
2) EOFException
3) Reader initialization error (failure of BufferedReader, or
DataInputStream, whatever is used in implementation)
....
n)

I'll repeat, one cannot change the signature of the method, for
instance, I cannot add *throws* declaration. So what would you do in
order to tell caller of an error? Please note that caller needs to
differentiate one error form another, so returning null is OK but only
for one type of error.
And one the most important things, that method must implemented in two
roles: RMI and File-System, what would be your suggestions taking into
consideration everything said above?

 

[Roedy Green]

So what would you do in
order to tell caller of an error?

you can throw an IllegalArgumentException or some other unchecked
exception.

 

[Jan Thomä]

Lets say you have the following interface:

Interface IFoo {
public String[] readStrings();
}
[...]
one cannot change the signature of the method, for
instance, I cannot add *throws* declaration. So what would you do in
order to tell caller of an error?

If the interface is under your control, you can define a set of Exceptions
that can be thrown by implementations on certain circumstances:

- MyFooIGotAnErrorWhileReadingException etc..

and add these to the signature. If the interface however is not under your
control, you have no other way than throwing unchecked exceptions
(Exceptions derived from RuntimeException), as Roedy Green already
mentioned.


Best regards,
Jan

 

[Andreas Leitgeb]

(Actually, the right choice would be to go back and change
the original interface definition, but that's not always
feasible.)

One example of where it is clearly infeasible is, when the
interface is actually not ones own work, like Runnable.run().

 

[Jan Thomä]

No other way, except to wrap the class in another class that throws the
exception, or to handle the exception invisibly such as by returning 'null' or
an empty array, or by using a completely different API that doesn't have the
restriction,

What do you bet that there is no other way than those?

Okay for the sake of nitpicking there are a bazillion other ways of doing
it. However I implied that

- the OP has to use the provided interface
- cannot change the provided interface

Therefore you can either hack something into the return value, which is
impractical as the caller cannnot find out what went wrong (as the OP
clearly stated) or you throw RuntimeExceptions which are not checked by the
compiler. Or you can invent some kind of messaging service into which the
called function pushes it's error code and the caller retrieves it, but i
thought we speak about practical solutions here...

Jan

 

[Owen Jacobson]

Okay for the sake of nitpicking there are a bazillion other ways of doing
it. However I implied that

 - the OP has to use the provided interface
 - cannot change the provided interface

Therefore you can either hack something into the return value, which is
impractical as the caller cannnot find out what went wrong (as the OP
clearly stated) or you throw RuntimeExceptions which are not checked by the
compiler. Or you can invent some kind of messaging service into which the
called function pushes it's error code and the caller retrieves it, but i
thought we speak about practical solutions here...

Jan

I don't think that's an entirely impractical solution:

public interface ExceptionHandler {
public void handleException (Exception e);
}

public class MyFoo implements IFoo {
private final ExceptionHandler eh;

public MyFoo (ExceptionHandle eh) {
this.eh = eh;
}

public String[] readStrings () {
try {
// Read from a file and possibly throw IOException
} catch (RuntimeException re) {
throw re; // propagate unchecked exc. untouched
} catch (Exception e) {
eh.handleException (e);
}
}
}

There are infinite variations on this; you could, for example, make
readStrings rethrow checked exceptions as RuntimeExceptions if eh is
null, or remove the bypass for unchecked exceptions and make
ExceptionHandler deal with Throwable instead of Exception.

Sure, it's clunkier than the natural solution: the exception handler
is lexically separated from the code that can throw an exception.
However, if the interface really is immutable, this might be the
cleanest way I can think of to retain the checked-ness of exceptions
without changing the interface.

For completeness, here's an ExceptionHandler that knows about
IOExceptions:

public class IOExceptionHandler implements ExceptionHandler {
public void handleException (Exception e) {
try {
throw e;
} catch (IOException ioe) {
System.err.println ("Handled an IOException: " + ioe);
} catch (Exception e) {
// Unexpected variety of failure here.
throw new RuntimeException (e);
}
}
}

As illustrated, the design of ExceptionHandler as written demands that
implementations be prepared for *any* Exception, even if the caller
will only ever pass some specific type of Exception. The design could
be improved by providing specialized ExceptionHandler interfaces for
different implementations of IFoo, such that each handler only has to
deal with the exceptions that can actually be thrown.

-o

 

[Jan Thomä]

I don't think that's an entirely impractical solution:

Well a good approach to implement the error handling, indeed. And probably
not even impractical.

Sure, it's clunkier than the natural solution: the exception handler
is lexically separated from the code that can throw an exception.

Indeed. Plus every implementation can come up with a different way of
implementing error handling which is probably not a good thing either. So
if the interface is specified that way the OP layed out, it would imply to
me that this function must not throw any exception in any case - or at
least it would imply that the caller does not need to expect any exceptions
to be thrown by any implementation. If this was not the case, the function
should specify which Exceptions are to be thrown on which conditions.

So since the implementation is free to do any error handling strategy that
the implementor deems good enough, i'd say wrapping any exception into a
runtime exception does the job for me. Otherwise the interface (== the
contract between me and the caller) should have stated something else.

Jan

 

[Patricia Shanahan]

you can throw an IllegalArgumentException or some other unchecked
exception.

I don't think it should be an existing exception. Rather, it should be a
class, extending RuntimeException, that is declared in the same package
as the implementing code. That way, there is no risk that some method in
the call stack throws the same exception with a specified meaning, and
it is being caught by something that method's caller.

The new exception can, of course, wrap an IOException etc. to preserve
all the details.

Patricia

 

[Owen Jacobson]

Or you could use 'if ( e instanceof IOException )', which preserves the stack
frame of the original exception and perhaps would be quicker, not that speed
matters to exception handling.

Rethrowing an exception does not, under the Sun JVM, clear its
stacktrace:

public class Foo {
public static void main(String[] args) {
Handler h = new Handler();

try {
throw new Exception();
} catch (Exception e) {
h.handle(e);
}
}
}

class Handler {
public void handle(Exception e) {
try {
throw e;
} catch (Exception ee) {
e.printStackTrace();
}
}
}

//--

Prints:

java.lang.Exception
at Foo.main(Foo.java:6)

The stacktrace is actually populated when the exception is created;
one of the problems with creating an Exception object in advance is
that the stacktrace will not indicate where it was thrown!

There are some clever uses for this behaviour that I will leave it to
you to discover.

Generics?

  public interface ExceptionHandler < E extends Exception >
  {
   public void handle( E e );
  }
...


Pretty much any time I see run-time type checking built into the code, I
think, "This is a job for generics!"

Well, a job for polymorphism primarily, and generics frequently.

That works for single exceptions (and it's probably how I'd do it
under the constraints), but not all services only throw one type of
exception that's more specific than Exception. Other approaches are
necessary if you want to make the exception handler interface specific
to the exceptions that can actually be thrown, for these services.

-o

 

[Patricia Shanahan]

Lew wrote:
....

Often the semantics of a method that returns a String [], as in the OP's
situation, or equivalently a Collection<String>, disregard the reason
why "nothing" is retrieved. All the client code really cares about is
if anything came back. An empty set is empty whether the database is
down or the customer never shopped there, who cares which? Either way
the situation could be different by the time of the next query. In that
case, a value like Answers.EMPTY (a final value from a hypothetical
Answers class) defined as a String [0] or an unmodifiable new
ArrayList<String>(0) would be the sentinel value returned.

I think this is very application dependent, and the OP needs to look at
the problem from the caller's point of view. For example, if I access my
bank's web site, I would rather get a general "unable to process your
transaction" than a denial that I have any accounts with them.

Patricia

 

[Andreas Leitgeb]

For example, if I access my bank's web site, I would rather
get a general "unable to process your transaction" than a
denial that I have any accounts with them.

.... even, if for some reason the bank believes that the latter
was the case ?

 

[Patricia Shanahan]

... even, if for some reason the bank believes that the latter
was the case ?

Yes, if that is the bank's considered opinion. No, if the web site has
just lost contact with the database, and has no idea whether I have
accounts or not.

Patricia

 

[Royan]

So since the implementation is free to do any error handling strategy that
the implementor deems good enough, i'd say wrapping any exception into a
runtime exception does the job for me. Otherwise the interface (== the
contract between me and the caller) should have stated something else.

That is a valid approach. I suggest that one remember what it "means" to have
an exception, what it means to throw an exception, and the prime importance of
operations, particularly relative to development. Jan's answer typifies this
deep understanding - notice the emphasis on what the *contract* of the
interface is, and the acknowledgment that that contract is by design.
Whatever the implementor does, it must not violate that contract, so it
behooves the author of an implementation to understand the semantics of the
contract.

Throwing a runtime exception in turn is also a signal to client code of the
rethrower. It tells that client code that it made a mistake, under the
programmer's control. "Tsk, tsk, you should not have given that illegal value
(IllegalValueException)." This type of error is for the client programmer, to
help them debug their code.

Checked exceptions tend to be for things the programmer cannot reasonably
control, like the file being deleted by an external process or the certificate
repository suddenly becoming unavailable. That's why the API designer placed
those exceptions in the contract for the method. Because the client
programmer cannot control these events, the API designer ensures that they
must check for them. Logging for this type of exception is vital - it helps
the sysadmin understand the operational fubar and fix it.

Often the semantics of a method that returns a String [], as in the OP's
situation, or equivalently a Collection<String>, disregard the reason why
"nothing" is retrieved. All the client code really cares about is if anything
came back. An empty set is empty whether the database is down or the customer
never shopped there, who cares which? Either way the situation could be
different by the time of the next query. In that case, a value like
Answers.EMPTY (a final value from a hypothetical Answers class) defined as a
String [0] or an unmodifiable new ArrayList<String>(0) would be the sentinel
value returned.

OK So let me summarize: There are three options:

1) Wrap checked exceptions and rethrow some sort of RuntimeException
that would contain some detailed message of an error
2) Retrieve some predefined, most likely constant value of type
String[] that any class can access and understand what has happened.
3) Create an exception handling method and handle all exceptional
cases there

Am I correct that there are no more solutions to the problem (assuming
we cannot change interface)?

If so I vote for option 2, because option 1 and 3 is evil. I'll
explain why, though again I'll just summarize arguments that have
already been outlined in this or that way above.

1) Wrapping exception is a bad idea since we break the semantical
meaning of the code. Utility method should not handle exceptions fired
because of its activity. It must simply handover the situation to some
other method. If it was not like that I doubt we would see every API
IO read/write method throwing IOException
3) Exception handler method that is used within utility method seems
to be even worse. First because caller should (in our case this is the
method where a call to readStream occurs) always receive information
about the success or failure of "readStrings()" operation. If we grab
every checked exception and return null indicating an error we will
never be able to properly separate view from model. For instance if
FileNotFoundException has occurred, I may want to visually tell user
of an error, but if i grab that exception an return null I will not be
able to understand if it was a FileNotFoundException or IOException or
whatever exception I will always receive null.

So what do you think? Do you concur with my arguments?

-R.

 

[Andreas Leitgeb]

Yes, if that is the bank's considered opinion. No, if the web site has
just lost contact with the database, and has no idea whether I have
accounts or not.

So, even if the bank's considered opinion is that you do not have an
account with them, then you'd still prefer a message like "unable to
process your transaction", but if it was just an issue with DB
connectivity, then you'd want to be told that "you do not have any
accounts with them" ?

Funny, I'd have chosen exactly the opposite ...

 

[Owen Jacobson]

OK So let me summarize: There are three options:

1) Wrap checked exceptions and rethrow some sort of RuntimeException
that would contain some detailed message of an error
2) Retrieve some predefined, most likely constant value of type
String[] that any class can access and understand what has happened.
3) Create an exception handling method and handle all exceptional
cases there

Am I correct that there are no more solutions to the problem (assuming
we cannot change interface)?

If so I vote for option 2, because option 1  and 3 is evil.
...

3) Exception handler method that is used within utility method seems
to be even worse. First because caller should (in our case this is the
method where a call to readStream occurs) always receive information
about the success or failure of "readStrings()" operation. If we grab
every checked exception and return null indicating an error we will
never be able to properly separate view from model. For instance if
FileNotFoundException has occurred, I may want to visually tell user
of an error, but if i grab that exception an return null I will not be
able to understand if it was a FileNotFoundException or IOException or
whatever exception I will always receive null.

The point I was making was to implement 3 by having the caller pass in
its desired exception handling strategy (via the constructor in my
example). It is equivalent to wrapping the method in an application-
specific try/catch block; the difference is in how it's implemented,
not in the functionality it provides.

So, for your case, you would pass in a handler (exception callback, if
you will) that knows to display a "file not found" dialog on
FileNotFoundException.

-o

 

[Jan Thomä]

OK So let me summarize: There are three options:

1) Wrap checked exceptions and rethrow some sort of RuntimeException
that would contain some detailed message of an error
2) Retrieve some predefined, most likely constant value of type
String[] that any class can access and understand what has happened.
3) Create an exception handling method and handle all exceptional
cases there


If so I vote for option 2, because option 1 and 3 is evil. I'll
explain why, though again I'll just summarize arguments that have
already been outlined in this or that way above.

1) Wrapping exception is a bad idea since we break the semantical
meaning of the code. Utility method should not handle exceptions fired
because of its activity. It must simply handover the situation to some
other method. If it was not like that I doubt we would see every API
IO read/write method throwing IOException

That is why they declare it. We talked about having a fixed method
declaration which cannot be changed. In that case the method was
ill-declared in the first place and should have declared the right way,
that is specifying the exceptions to be thrown and catched. Since we cannot
redeclare it, i go for option 1 because:

2) Worst of all for me, because you can easily forget to handle this
special return value and your code will run all the time you code it
(because the error never happens), it will run through all your test cases
but it will blow when being in production for the first month and you will
not even have an exception stacktrace but just a program that produces
weird behaviour with no idea why it does that way (you can search for ages
on such a thing, I've had that to the limit). Throwing an exception is the
best way to make sure that errors are handled, because you either handle
them or you got a nice big stacktrace in your error logs and your client
will surely ask you about it (well my client at least would do so).
Additionally you might have a hard time finding a special return value at
all which will not be returned when the function exits normally (e.g.
assume a function that does a name lookup and returns null when no such
name can be found - how do you handle an exception like "i had no database"
when the null you got could result from normal operation as well?).

3) Exception handler method that is used within utility method seems
to be even worse. First because caller should (in our case this is the
method where a call to readStream occurs) always receive information
about the success or failure of "readStrings()" operation. If we grab
every checked exception and return null indicating an error we will
never be able to properly separate view from model.
For instance if
FileNotFoundException has occurred, I may want to visually tell user
of an error, but if i grab that exception an return null I will not be
able to understand if it was a FileNotFoundException or IOException or
whatever exception I will always receive null.

Which however is more or less the thing you suggest in option 2) Except
for the fact that you can get the error message from somewhere, which you
cannot do when doing plain option 2). So its still not good but better than
option 2), i think.


Anyways, i suppose most of the important stuff has been said to this point.
I assume, that we all can agree that a proper error handling strategy needs
to be tailored to the problem and that there is no silver bullet in doing
so. One has to do some tradeoffs when being faced with facts that one
cannot change (like an ill-designed external library), and making a choice
is not always easy, so asking for a good strategy is never a bad idea. I
liked the discussion very much

Jan

 

[Royan]

OK So let me summarize: There are three options:

1) Wrap checked exceptions and rethrow some sort of RuntimeException
that would contain some detailed message of an error
2) Retrieve some predefined, most likely constant value of type
String[] that any class can access and understand what has happened.
3) Create an exception handling method and handle all exceptional
cases there

If so I vote for option 2, because option 1 and 3 is evil. I'll
explain why, though again I'll just summarize arguments that have
already been outlined in this or that way above.

1) Wrapping exception is a bad idea since we break the semantical
meaning of the code. Utility method should not handle exceptions fired
because of its activity. It must simply handover the situation to some
other method. If it was not like that I doubt we would see every API
IO read/write method throwing IOException

That is why they declare it. We talked about having a fixed method
declaration which cannot be changed. In that case the method was
ill-declared in the first place and should have declared the right way,
that is specifying the exceptions to be thrown and catched. Since we cannot
redeclare it, i go for option 1 because:

2) Worst of all for me, because you can easily forget to handle this
special return value and your code will run all the time you code it
(because the error never happens), it will run through all your test cases
but it will blow when being in production for the first month and you will
not even have an exception stacktrace but just a program that produces
weird behaviour with no idea why it does that way (you can search for ages
on such a thing, I've had that to the limit). Throwing an exception is the
best way to make sure that errors are handled, because you either handle
them or you got a nice big stacktrace in your error logs and your client
will surely ask you about it (well my client at least would do so).
Additionally you might have a hard time finding a special return value at
all which will not be returned when the function exits normally (e.g.
assume a function that does a name lookup and returns null when no such
name can be found - how do you handle an exception like "i had no database"
when the null you got could result from normal operation as well?).

3) Exception handler method that is used within utility method seems
to be even worse. First because caller should (in our case this is the
method where a call to readStream occurs) always receive information
about the success or failure of "readStrings()" operation. If we grab
every checked exception and return null indicating an error we will
never be able to properly separate view from model.
For instance if
FileNotFoundException has occurred, I may want to visually tell user
of an error, but if i grab that exception an return null I will not be
able to understand if it was a FileNotFoundException or IOException or
whatever exception I will always receive null.

Which however is more or less the thing you suggest in option 2) Except
for the fact that you can get the error message from somewhere, which you
cannot do when doing plain option 2). So its still not good but better than
option 2), i think.

Anyways, i suppose most of the important stuff has been said to this point..
I assume, that we all can agree that a proper error handling strategy needs
to be tailored to the problem and that there is no silver bullet in doing
so. One has to do some tradeoffs when being faced with facts that one
cannot change (like an ill-designed external library), and making a choice
is not always easy, so asking for a good strategy is never a bad idea. I
liked the discussion very much

Jan

Thanks Jan, and everyone who has taken part in the discussion, I agree
with Jan that everyone can make up his/her mind now and decide which
solution is better.