[coding practises] do you declare your superclass "throws"??

[jrobinss]

I've just discovered something I didn't know about.

If I have, for example, class javax.servlet.http.HttpServlet.
I make my own servlet, MyServlet.
I overload doGet() thus:

@Override
protected void doGet(HttpServletRequest request,
HttpServletResponse response)
throws ServletException, IOException {
[...]

But then I get a warning, because I don't throw any ServletException
in my code. I checked the JLS, and there it was,I can't declare *more*
exceptions in the sub-class... but I can declare *less*.
(see http://java.sun.com/docs/books/jls/third_edition/html/classes.html#8.4.6)
So yes, the warning is justified.
(even if I didn't understand everything about erasure, and when
there's supposed to be a compile-time error or unchecked warning)

But this got me thinking... In future editions of the code, by other
members of the team, in a year or two, will anyone be aware that they
may throw ServletException but not any other one?
Isn't it useful to know that this method could -- if required -- throw
this particular exception?

My question is:
how do you manage this, in your projects or teams?
Do you declare all inherited "throw" statements (including unused
ones) or do you declare only the necessary ones?

Remark: of course, declaration of useless exceptions is annoying to
the caller, but in this kind of case the caller is bound to call a
servlet, not my specific servlet, so the caller is bound to call the
superclass's method, with all the possible throw statements.

Funny issues.

 

[Mark Space]

But this got me thinking... In future editions of the code, by other
members of the team, in a year or two, will anyone be aware that they
may throw ServletException but not any other one?

If they try, the compiler will remind them and stop. In other words, if
they throw IOException at any point in the future, the compiler will say
something like:

"You didn't say throws IOException on this class"

And programmer will say:

"Huh, better add that."

And that's as much time as it will take. So in summary, this is not a
maintenance issue. Remove the IOException from the throws clause, and
don't worry about it.

 

[Andreas Leitgeb]

I've just discovered something I didn't know about.
@Override
protected void doGet(...) throws ServletException, IOException {
But then I get a warning, because I don't throw any ServletException
in my code.

I just wrote a simple test-case:
class X {
public static int foo() throws IOException { return 0; }
}

and compiled it with javac 1.5.0_09 and with 1.4.2_08
and never did it write any error or warning.

Is there some fundamental difference between a ServletException
(which I do not have available here for testing) and the IOException
that I missed? Both seem to be derivatives from java.lang.Exception.

Or did I just misunderstand the problem?

What indeed does cause an error is an explicit
try-catch for an exception that isn't actually
thrown. This is, however, only annoying during
development, when temporarily commenting out the
actual throwing part, and then having to also
comment out the try-catch.

 

[jrobinss]

On 18 jan, 11:24, Andreas Leitgeb <[email protected]>
wrote:
[...]

and compiled it with javac 1.5.0_09 and with 1.4.2_08
and never did it write any error or warning.

Sorry, I should have said, it's Eclipse who's generating warnings.
These are configurable, and not so much linked to the language specs
as a kind of help to write better code. It detects stuff like unused
imports statements or variables... or useless throw statements.
Anyway, the warnings themselves are not the issue, it's more what's
causing them.

I realize I badly explained the actual issue. For example Mark's
answer is beside the point (sorry!). Of course anyone needing a new
throw will be "helped" (in a forceful way) by the compiler to declare
what's needed and authorized.

The idea is that there's this code, where class B extends class A,
method B.n() extending method A.m(). A.m() throws MyException, B.n()
doesn't need to, so its declaration does not include the throw
statement.
Arrives a new coder, who needs to throw an exception, because there's
now a new call in the body of B.n() that raises an exception. The new
coder will not be authorized to add an exception to the method,
because it's overloading A.m(), so most likely s/he will add an
unchecked exception (RuntimeException, or something like that).
What's a shame is, the new coder could have used that MyException.
Only it wasn't apparent, because it was declared nowhere in B. New
coder could have gone to class A, but after all, when editing the
implementation of a class you don't always check all superclasses and
interfaces for possibilities (or do you?). And sometimes they are
hidden away in a jar and you don't have the code, so it's not easy to
browse them.

As an example, new coder could write...

/**
* note from new coder: would like to throw ServletException, but can't
because it's an overload
*/
@override
public void someMethod() {
try {
doStuffThatThrowsServletException();
} catch (ServletException se) {
throw new RuntimeException("", se);
}
}

....when it would have been possible to write...

/**
* note from new coder: I added ServletException!
*/
@override
public void someMethod() throws ServletException {
doStuffThatThrowsServletException();
}


I realize while writing this that most people will probably advise me
to write off the useless throw statements and trust further coders to
do a minimal research job. I simply wish coding was so simple,
personaly I see so much approximate coding everywhere that I like
guidelines.
Adn I wanted to know what other programmers were doing.
(which is what NGs are for, ain't it? )

Hoping to be more clear!

 

[Andreas Leitgeb]

Sorry, I should have said, it's Eclipse who's generating warnings.

The idea is that there's this code, where class B extends class A,
method B.n() extending method A.m(). A.m() throws MyException, B.n()
doesn't need to, so its declaration does not include the throw
statement.

I take it that the override in B of course also must be name "m"

I understand your being concerned about future developers, who might
not know, that adding ServletException to that particular method is
ok, and I want to add an even stronger motive to leave the S.E.
in the throws-list:
A future programmer might want to derive his class C from your class B,
and then feel the need to throw a S.E. from his C.m . *There* he can
no longer add the exception to the throws-list.

I realize while writing this that most people will probably advise me
to write off the useless throw statements and trust further coders to
do a minimal research job.

No, quite the opposite: If you have a feeling that either your method,
or any method overriding yours may have a legitimate point for throwing
a ServletException, you really should list it in your method's throw-
list, but don't feel overly responsible for yet unknown derive class'
needs. As a rule of thumb, leave in those exceptions that are not
semantically at odds, with what your class is designed for.

If a derived class *unexpectedly* needs to throw ServletExceptions, it
can still pass it as rootCause to the IOException If your class
doesn't deal with Servlets (e.g. doesn't even call baseclass' method)
and is not directly designed to accomodate for Servlet-related
sub-classes, then eclipse's advice probably applies.

Configure that warning away in eclipse if it bugs you. Eclipse is
trying to help, but if you know better after having considered
Eclipse's advice, then eclipse's advice is the problem, and should
be solved inside eclipse rather than worked around in java-code.

 

[jrobinss]

Thanks a lot for a useful and insghtful response.
(as they say in French, "tout flatteur vit aux dépens de celui qui
l'écoute", ha ha)

I take it that the override in B of course also must be name "m"

Well, in fact, say that to the authors of JLS, whom I took this naming
from. (nah, in fact they don't use method name m and n, they call the
methods m and n, which is a semantic nitpick but answers your
comment).

A future programmer might want to derive his class C from your class B,
and then feel the need to throw a S.E. from his C.m . *There* he can
no longer add the exception to the throws-list.

Good point.

As a rule of thumb, leave in those exceptions that are not
semantically at odds, with what your class is designed for.

I agree.

In fact, this more or less means that as a rule of thumb, ignore
Eclipse's warning that this "throws" is useless... Eclipse knows only
about the implementation, not the design.

Thanks

 

[Lew]

Thanks a lot for a useful and insghtful response.
(as they say in French, "tout flatteur vit aux dépens de celui qui
l'écoute", ha ha)



Well, in fact, say that to the authors of JLS, whom I took this naming
from. (nah, in fact they don't use method name m and n, they call the
methods m and n, which is a semantic nitpick but answers your
comment).


Good point.


I agree.

In fact, this more or less means that as a rule of thumb, ignore
Eclipse's warning that this "throws" is useless... Eclipse knows only
about the implementation, not the design.

Also this concern for "future developers who may want to ..." is beside the
point. If you think the class you're writing is going to need
ServletException thrown by that method later, why would you remove it now?
The idea is to write the code to avoid the need for later modification if
possible - make it reusable rather than easily modifiable.

The thing that is easiest for future developers isn't to be able to change
your code, it's not to need to.

 

[Lew]

Also this concern for "future developers who may want to ..." is beside
the point.

s/beside/exactly/

 

[Andreas Leitgeb]

Also this concern for "future developers who may want to ..." is beside the
point. If you think the class you're writing is going to need
ServletException thrown by that method later, why would you remove it now?

Whether it will be a future developer, or oneself at a later time
generally turns out to be principially the same thing: Even oneself
then likely has forgotten, whatever the other future developer just
doesn't know.

It was about weighing the quality of eclipse's advice (to remove
exceptions from throws-clause that are at the current moment not
thrown). The answer being: it depends on things that eclipse
cannot know.

 

[Lew]

Whether it will be a future developer, or oneself at a later time
generally turns out to be principially the same thing: Even oneself
then likely has forgotten, whatever the other future developer just
doesn't know.

It was about weighing the quality of eclipse's advice (to remove
exceptions from throws-clause that are at the current moment not
thrown). The answer being: it depends on things that eclipse
cannot know.

Indeed, and I agree with your conclusions.

My strategy for being kind to that future person is to follow your advice in
the case of the overridden method signature retaining ServletException, since
it's part of the expectation from the superclass. This guarantees that use of
my custom subclass will be consistent with that of its superclass whether
further extended or no. This is consistent with theoretical foundations like
Liskov substitution.
<http://en.wikipedia.org/wiki/Liskov_substitution_principle>

In this case, I'm extending the notion to the class itself, imputing a
property q(T) that one can extend T with a further subclass, X, and declare
the overridden method to throw ServletException. A subclass S that does not
include ServletException in that method's signature would not be substitutable
in the 'extends' clause of X.

 

[Andreas Leitgeb]

My strategy for being kind to that future person is to follow your advice in
the case of the overridden method signature retaining ServletException, since
it's part of the expectation from the superclass.

I wouldn't go as far as making it a general, all-time-valid rule.

If I had a class that wraps integers, and a method that was only usable
on odd integers and threw an EvenException for even ones, then a subclass
dealing with only odd integers would remove the EvenException from its
overrided method's throws-clause, because it makes no sense for that class,
nor for any further subclass of it

This is consistent with theoretical foundations like Liskov substitution.
<http://en.wikipedia.org/wiki/Liskov_substitution_principle>

Interesting concept, but I think you're overstretching it to a point
were rendering it a strict equality relation is not far away, thus
voiding the whole subclass-concept.

In this case, I'm extending the notion to the class itself, imputing a
property q(T) that one can extend T with a further subclass, X, and declare
the overridden method to throw ServletException.

That's like: some q(Object) is "can be a String or an Integer."
This property is not retained by Integers, so Integers aren't
really a subtype of Object ;-)

 

[Ben Phillips]

I just wrote a simple test-case:
>
class X {
public static int foo() throws IOException { return 0; }
}

and compiled it with javac 1.5.0_09 and with 1.4.2_08
and never did it write any error or warning.

This is interesting. It could stand to give a warning in this case, or
where it's final or private. The rest of the time, no error or warning
seems best, since even if you don't throw it yourself you may be
intentionally declaring that overridden versions may throw it.

What indeed does cause an error is an explicit
try-catch for an exception that isn't actually
thrown. This is, however, only annoying during
development, when temporarily commenting out the
actual throwing part, and then having to also
comment out the try-catch.

Very annoying. This one should be a warning rather than an error, IMO.
Unreachable code in general should be.

 

[Lew]

This is interesting. It could stand to give a warning in this case, or
where it's final or private. The rest of the time, no error or warning
seems best, since even if you don't throw it yourself you may be
intentionally declaring that overridden versions may throw it.

Not with static methods, because one cannot override a static method.

Very annoying. This one should be a warning rather than an error, IMO.
Unreachable code in general should be.

Unreachable code should be what? Should be a warning rather than an error?

Java's philosophy tends to be to check as much as can be checked at compile
time. Your feelings about what "should be" will have to concede to what is.

Java does have a Request for Enhancment (RFE) process.

Some IDEs, such as Eclipse, let you adjust the severity of some of their
internal compiler messages (e.g., make a warning rather than an error).

 

[Ben Phillips]

Not with static methods, because one cannot override a static method.

That was my point -- it could stand to give a warning with static
methods and with private or final ones, though it shouldn't with
overridable methods.

Unreachable code should be what? Should be a warning rather than an error?

Yes, since it's not actually incorrect and since making it an error
causes inconvenience. Since it is "strange" or a potential sign of
trouble, it shouldn't be silent either; hence a warning, which is what's
traditionally used for things that may be OK but may be a sign of a
programming logic error.

Some IDEs, such as Eclipse, let you adjust the severity of some of their
internal compiler messages (e.g., make a warning rather than an error).

This is quite useful, yes.

 

[Lew]

Yes, since it's not actually incorrect and since making it an error
causes inconvenience.

I'm afraid that it is actually incorrect, in that it violates the Java
Language Standard.

<http://java.sun.com/docs/books/jls/third_edition/html/statements.html#14.21>

It's just as incorrect as any other violation of the Standard.

The law is the law.

 

[Andreas Leitgeb]

I'm afraid that it is actually incorrect, in that it violates the Java
Language Standard.
<http://java.sun.com/docs/books/jls/third_edition/html/statements.html#14.21>

This is a circular, since this part of the JLS is the very point
we(Ben and me) are criticizing.

The "law" appears to me like a state-law against "silly walks". Not that
the latter were all that useful, but why forbid them?

 

[Lew]

This is a circular, since this part of the JLS is the very point
we(Ben and me) are criticizing.

The "law" appears to me like a state-law against "silly walks". Not that
the latter were all that useful, but why forbid them?

It's not circular. You are essentially asking a telepathy question. Because
your esthetic is somehow offended by a rule that eliminates by design a
useless and possibly maintenance-damaging wart from programs that are
/supposed/ to be durable, you guys whine that it's "inconvenient" and try to
second-guess the intent of the designers. The fact is that it clearly /was/
the intent to prevent useless extra unreachable code, and that formal
reasoning about the program would be eaiser for the compiler if this became a
language requirement. The language is defined by its very rules - accusing me
of "circular" reasoning by reference to those rules is specious.

The Java designers clearly felt that this was a fundamental and important
issue - the section on unreachable code is one of the most detailed and
rigorous in the JLS. Clearly they felt it was vitally important, as you will
discover if you read that part. It seems to my eye that it's one of the
fundamental things that make Java what it is, based on how much care they put
into the matter.

Now you apparently disagree with that reasoning. That is fine. But that
doesn't make you right, or change the fact that reachability is one of the
fundamental design points that make Java the language that it is. Java is
designed to avoid problems, not invite them, and to create durable software
that will be maintainable.

Really the argument should be on the other side - why should unreachable code
be tolerated at all? There must be a name for the logical fallacy that throws
the burden of proof on the wrong party.

 

[Ben Phillips]

I'm afraid that it is actually incorrect, in that it violates the Java
Language Standard.

You misunderstand me. I don't mean to say that it doesn't violate the
Java Standard. I mean to say that having dead code doesn't logically
prevent the program from being compilable and runnable; the standard
could hypothetically be amended to allow dead code without the result
being that there was an irredeemable ambiguity or other problem with
implementing a compiler to follow the changed standard, or a breakdown
of any of the core of Java's "social contract" with coders and users,
such as that it is fairly strongly type-safe, memory/pointer-safe, and
secure in particular ways (JNI and reflection shenanigans, including
hex-editing serialized objects, notwithstanding).

In fact, arguing based on what the standard currently says in a debate
about whether an aspect of the standard itself is optimal seems a little
bit circular to me...

 

[Ben Phillips]

It's not circular.

Sure it is.

> You are essentially asking a telepathy question.
Because your esthetic is somehow offended by a rule that eliminates by
design a useless and possibly maintenance-damaging wart from programs
that are /supposed/ to be durable, you guys whine that it's
"inconvenient" and try to second-guess the intent of the designers.

But there's now at least one documented case where the standard *causes*
inconvenience. Right now you sometimes can't test a change or comment
out one line of code without commenting out a whole block of code
somewhere else, AND another line or two in other places, in a manner
that is somewhat awkward. For example one line in a try block being
commented out or a change there tested may require commenting out the
try line, some distance above, and an entire catch block below,
ultimately forming three disjoint pieces of code. Easy to miss that
these three commented-out regions should really be commented back in
together as a single unit later on. Also more work. Some other cases
like this are logically unavoidable: if you comment out a declaration
you will clearly need to comment out every use of the declaration. The
try...catch case is not inherently unavoidable this way. It occurs
because of an arguably-gratuitous "feature" of the standard rather than
inevitable logic. Declarations are also not as often commented out or
experimented with in this sort of manner. The only cases I've ever run
into that've annoyed me have involved try...catch and switch, the latter
when the cases become or stop being exhaustive and the default sometimes
needs commenting out.

A warning suffices to alert that this code is not polished enough for
release, as a general rule.

Also, vast amounts of dead code that confound maintenance programmers
take the form of entire methods and even whole classes that lie about
collecting dust. None of these seems to generate an error with the
javacs I've used. In the non-private case, the compiler can't be sure
the code won't be linked with code that calls these methods, though, so
there is nothing it can do. I do seem to recall private methods that
aren't used generating a warning with one, but not an error.

Requiring dead code to be commented out would be reasonable if it were
easy to do so without the problems described above for try...catch.
Default blocks and entire unused private methods are easy to comment out
because they are single contiguous chunks of code. Commenting out a
catch is another matter. Perhaps a compromise would be to allow a
nilpotent try block -- a try { stuff } with no catch or finally clauses.

I suppose a workaround is to have a dummy finally clause containing only
a comment on every try block that doesn't have a "real" finally clause.
Then you can comment out a catch block without a complaint, even if it's
the sole catch block. This saves commenting out the try line. It still
means commenting in or out the throw in the try body and the catch
clause as a unit, but at least you will generally get an obvious
reminder to keep them in synch as the throw without the catch will, in
many cases, result in the compiler complaining that it's not declared or
handled (checked exceptions not thrown by the method being edited) and
the catch without the throw will get the current dead-code complaint.

Dummy finally clauses seem ugly to me, though, and a way to clutter code
with unhelpful and gratuitous crud -- the exact sort of thing the
standard here is argued to be trying to reduce.

Another compromise would be to permit an empty catch clause for an
unthrown exception. Then you could comment out the catch block's innards
and again save having to comment out the try line.

Warnings make sense in all of these cases (any try with no catch or
finally blocks that contain actual code) both to catch this sort of
temporary hack before release so it can be removed or otherwise fixed
(e.g. by uncommenting code you forgot to uncomment-out in a catch block)
and to catch some unrelated and common bad coding practises, such as to
catch and discard actually-thrown exceptions without taking any action.

This last assumes that a shop has a policy of not shipping as final (or
even beta) any code that generates warnings, and likewise requires real
justification for every @SuppressWarning use in production code. I hope
that that is considered basic common sense in every Java shop though.

formal reasoning about the program would be
eaiser for the compiler if this became a language requirement.

This is just plain wrong. To support the current standard, the compiler
has to determine which code is provably-dead-by-static-analysis and then
complain if there is any dead code detectable in this manner. The formal
reasoning in question can be supported if the compiler allows dead code
at the cost of simply performing the same static analysis and then
ignoring whatever it detects to be dead code (other than to emit a
warning). In other words, the compiler is required by the standard to
perform the same analysis that is needed anyway. Formal reasoning is
exactly as easy (or hard) either way.

> The
language is defined by its very rules - accusing me of "circular"
reasoning by reference to those rules is specious.

Not when those rules themselves are the subject of discussion. If we
were discussing whether some construct was legal in current Java, appeal
to the rules would not only be valid but the obvious way to definitively
answer the question. But we were not; we were discussing whether or not
it is desirable to *change* one of those rules, and there arguing that
it's not because the rules are the rules amounts to declaring by fiat
that the status quo is invariably preferable to any change, without
proving this. Such an attitude would, if held by Sun itself, have us
still using the clunky for (Iterator x = y.iterator(); x.hasNext() to
loop over collections. (And lacking the type-safe generics we now enjoy,
but I think less people are enamored of generics as of the new for loop. )

The Java designers clearly felt that this was a fundamental and
important issue - the section on unreachable code is one of the most
detailed and rigorous in the JLS. Clearly they felt it was vitally
important, as you will discover if you read that part.

That is not in dispute. Whether they actually were correct is under
discussion, not whether they felt strongly that they were correct.

Now you apparently disagree with that reasoning. That is fine. But
that doesn't make you right

Nor does the existing wording of the JLS automatically make us wrong.

or change the fact that reachability is one
of the fundamental design points that make Java the language that it
is. Java is designed to avoid problems, not invite them, and to create
durable software that will be maintainable.

I fail to see how warnings don't suffice, given a reasonable policy of
not letting code that generates warnings (or use of @SuppressWarning to
hide problems instead of solely in the rare and necessary cases
involving interfacing generics to arrays or legacy ungenerified code)
get out of alpha until the causes of the warnings are gone.

Really the argument should be on the other side - why should unreachable
code be tolerated at all? There must be a name for the logical fallacy
that throws the burden of proof on the wrong party.

As far as I'm aware, there is none. Nor is there any clear indication
that the burden of proof *is* on the wrong party here. Nobody is
asserting anything here entirely without evidence; you've pointed to the
standard, while we've pointed to an annoyance we've observed in actual
practise that may warrant amending that standard. Reducing the error to
a warning for the specific case of a catch clause for an unthrown
exception would be a fairly narrow and specific change and doesn't seem
wholly unjustified.

Perhaps the issue will become moot with smarter IDEs. Eclipse is already
surprisingly smart about some things, and so is NetBeans. Before long it
might be able to automatically comment out a catch block for an
exception no longer thrown, comment out the try line if there are no
"live" catch or finally clauses, and comment these back in if one of the
exceptions becomes thrown again, recognizing commented-out try lines and
catch blocks as something to uncomment if a matching exception throw
suddenly (re)appears between the try and the catch in question.
Essentially it would just require it to fix dead-code errors itself by
commenting out the offending code and the "other end" of the same brace
structure if there is one, and fix unhandled-exception errors by finding
a commented out catch that can handle it and uncommenting it (and any
matching commented-out try line, the nearest above it and at the same
nesting level) if possible.

Even a manual comment-out-clause and uncomment-clause command pair that
is smart enough to balance braces would improve this case a lot.

And ultimately it probably IS easier and likelier for the IDEs to change
than the JLS to change. Then again, Java 7 is apparently in the works
and fairly radical changes like reified generics are being contemplated.
Now may be a time when JLS changes are a bit more feasible.

It remains to question whether the suggested change would be desirable.
I'm not 100% sure myself.

 

[Andreas Leitgeb]

Default blocks and entire unused private methods are easy to comment out
because they are single contiguous chunks of code.

No, not even these are, since if the currently commented out line
(or block or method) happens to contain the one and only call to
yet another private method, then I'd enter a chain of methods to
comment out each separately, and re-activate them when re-activating
that one initial line/block/method.

Otoh, if rather than commenting stuff out I prepend it with "if (false)",
then the code is still dead, and the compiler doesn't even write a
warning.

But "if (false)"ing is still no substitute, since it still requires
the thusly deaktivated part to still be compileable.

Also, I just tried: unused private methods do *not even* raise
a warning in javac (this seems to be an eclipse-feature). I consider
unused private methods to be the principially same thing as unused
default-blocks in exhaustive switches and catches for unthrown
exceptions. Actually, if/while-statement-bodies, whose condition
can be determined as always-false could also be determined as
dead code, but only if the consequence were not worse than a warning,
since that's probably the most-used workaround for the annoying
feature under discussion.

I'd find it reasonable that all of them threw warnings.

But then again, maybe we're indeed barking up the wrong tree, and
what we really want is a compiler that, based on some options,
deviates from JLS to not overly annoy for code still under active
development.

I can agree to both Lew and JLS in that code that is shipped should
not contain any such dead blocks.