There have been a few messages recently regarding build warnings that
complain about e.g. local variables not being used or missing
annotations, and work going on to 'fix' them.
However, in most cases, the only thing that is being 'fixed' is that
the compiler warnings no longer appear. Realistically, it's not a
problem in the code; in fact, the code works fine with those warnings
in place. Arguably, therefore, the code is being fixed to suit the
compiler, rather than the compiler being configured to be more useful
(i.e. less verbose).
Of particular beef are the kind of automated clean-ups that IDEs such
as Eclipse will offer. These are mostly incredibly dangerous, and
offer no significant benefit to Harmony as a whole. Some of them are
benign (for example, updating the list of imports) and some of them
aren't (deleting private methods, unused variables etc.). The problem
is that unless you've got a good idea of the code at any one point,
performing automatic transformation of the codebase is not a sensible
plan.
Whilst the fact of the matter is true that it doesn't affect code
execution (because private methods *aren't* being called, or a local
variable *isn't* being read), it doesn't make sense to do it
necessarily. For example, one could argue that removing all the
JavaDoc from the source files has no effect on the code, and has the
added advantage that the size of the source code is smaller -- but
that doesn't make it a sensible thing to do.
In particular, there's a lot of areas in the code that are pretty
dynamic, and are still evolving. For example, I could be writing
something like:
int i;
i=someMethod();
and then want to commit it to the repository. Let's say I discover a
bug in someMethod(), so I instead do:
int i;
//i=someMethod() //TODO Fix the bug in someMethod()
Now, automated tools will complain that i is being unreferenced
(possibly even uninitalised too :-) and volunteer to take it out. The
problem is then, after I've committed and worked on my code, I have:
int i;
i=someMethod() // Fixed bug
and someone else has committed Auto-Delete, so that when I update it becomes:
i=someMethod() // Compiler error, i doesn't exist
The problem is that the person who ran Auto-Delete only saw a snapshot
of the source code. Yes, *at that point in time* the variable wasn't
being defined, but by removing it, you've effectively removed it from
versions in the future, too. It's even worse with a private method,
because then you've lost a lot more than a simple variable
declaration.
The same thing is true of the Auto-Generics that some people have
become fond of. Really, get over it: generics don't save anything that
you wouldn't have in code anyway (they're pretty much all compile-time
restrictions anyway; there's still no type safety at run-time like
there is in C#). Worse, Generics can only be used with Java 5 onwards,
so by introducing generics into a library you're preventing that same
library being used on anything older than Java 5. Granted, most of
Harmony is working towards the Java 5 goal, but I'm sure there will be
reusable parts of the codebase that other users would otherwise be
able to use on older VMs, such as some of the tools (keytool,
jarsigner) or of course, the pack200 libraries that I'm involved with
at present.
I've also no idea what IDE suggested this one, but in the following:
if(condition) {
// some code
return value;
} else {
// some other code
return otherValue;
}
is absolutely *not* the same as
if (condition) {
// some code
return value;
}
// some other code
return otherValue;
These two may execute the same, but they have very different
semantics. In the first example, it's impossible to go from the if
block to the else block in the future. However, in the second example,
it may be that you can wrap some extra conditions around the return
statement. In this case, they have different semantics:
if (condition) {
// some code
if (anotherCondition)
return value;
} else {
// some other code
return otherValue;
}
and
if (condition) {
// some code
if (anotherCondition)
return value;
}
// some other code
return otherValue
The point is that code has been written in one way (or other) for a
reason. The code may well evolve in the future (or be evolving as you
are currently applying automated fixes). If you know that the variable
is not only unused now, but always will be unused in the future, then
fine, remove it. However, if all you know is the variable is unused
now, then automated fixing can be detrimental to the health of the
codebase.
Alex.
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
