@Override everywhere
borges May 17, 2011 3:31 PMHi folks,
HornetQ trunk is now compiled using Java6. Since Java6, it is possible to add @Override also to methods overriding interfaces (that includes interface methods overriding other interface methods). I would like to make the case for us to start consistently using @Override also for interfaces.
At Effective Java, Josh Bloch makes the following points about Override:
Therefore, you should use the Override annotation on every method declaration that you believe to override a superclass declaration. There is one minor exception to this rule. If you are writing a class that is not labeled abstract, and you believe that it overrides an abstract method, you needn’t bother putting the Override annotation on that method. In a class that is not declared abstract, the compiler will emit an error message if you fail to override an abstract superclass method. However, you might wish to draw attention to all of the methods in your class that override superclass methods, in which case you should feel free to annotate these methods too.
[ ... snip lots of good text ...]
[...] it is worth annotating all methods that you believe to override superclass or superinterface methods, whether concrete or abstract.
(Joshua Bloch, Effective Java 2nd Edition, Item 36 "Consistently use the Override annotation") ( <-- read this book!)
I would like to make the case that we should also use override even for concrete classes overriding interface methods (the minor exception to the rule Bloch talks about).
[...]
There are correctness cases for using override:
- to avoid not overriding a method
- to avoid unintencional method override
1. Avoid not overriding a method, is easy to illustrate (and HornetQ code already practices it):
class B extends A {
public void doSomething() { // etc }
}
class A {
public void doSomeThing() { //etc }
}
2. avoid unintentional method override, is the use case improved with Java6.
This does not appear in new code implementing interfaces, but it becomes a problem when code quantity and complexity increase, and people start refactoring it
We have class A implementing (or not) some interfaces, and with ~ 2000 loc.
class A implements Foo, Bar {
[1000 loc]
public void doFooThis() { // overrides a method from Foo }
public void doThis() { // an innocent public non-overriding method }
[1000 loc]
}
This class now needs to implement interface Baz with a bunch of methods, one of which is void doThis() (!!)
interface Baz {
[ ... bunch of methods ... ]
void doThis();
}
class A implements Foo, Bar, Baz {
[1000 loc]
public void doFooThis() { // overrides a method from Foo }
public void doThis() { // an accidentally overriding method. NO WARNING! }
[1000 loc]
// A bunch of new methods are added...
}
Now we had a name collision, the developer was in a hurry, and neither the IDE nor the compiler could save us.
A related operation may occur when trying to split a class, say we need to split class A into (1) A implements Bar, Baz and (2) AFoo implements Foo.
class A implements Bar, Baz {
[1000 loc]
public void doFooThis() { // left over method from Foo. NO WARNING! }
public void doThis() { // a public accidentally overriding method }
[1000 loc]
// A bunch of new methods are added...
}
class AFoo implements Foo {
// code...
}
In both cases, if we had all methods annotated with override, we could have spotted the issues.
I know people are supposed to pay attention to code changes, but just like we need to add unit tests that do sanity tests, IMHO we should have @Override annotations in places that make sense.
[...]
In the short foreseeable future, I intent to add a checkstyle configuration to the project, and add Hudson reports for it. If you folks agree, I will turn on the checks for @Override for interfaces there (and in the default Eclipse configuration as well).