OK here's a lot more of my notes as I'm run analysis on the code:
new Integer(foo).toString() is better written as Integer.toString(foo)
Integer.parseInt() returns and int; Integer.valueOf() returns an
Integer. Likewise for other primitives. Use the one you need to avoid
an unneeded box or unbox.
Strings should always be compared with equals(), never ==
Never call System.exit(); it's nearly as harsh as "kill -9". Throw an
exception out of main.
StringBuilder is good but I've seen this:
StringBuilder foo = ...;
foo.append("something " + bar + " something else");
.. which rather misses the point of StringBuilder. Append each string
with append()
foo.append("something ").append(bar).append(" something else");
You can append a character like '\t' instead of a one-character String
like "\t". It's faster.
Don't use Vector or Hashtable or Enumeration anymore -- use ArrayList
and HashMap and Iterator, and wrap in Collections.synchronizedList()
et al. if you need the same synchronization semantics.
Javadoc comments start with /** and end with */ or else they aren't
recognized as such. Also I personally feel we should not write empty
javadoc or purely perfunctory javadoc that says nothing beyond what
the method already indicates -- e.g. "Gets foo" for getFoo()
I personally avoid imports using a wildcard (e.g. java.util.*). Though
I think this is a common convention -- not using wildcards -- I won't
change it here myself.
I also like to declare everything final that can be, except method
params and locals, where it makes for too much noise. But I won't
foist that on the whole codebase. Likewise I prefer to make elements
as private as possible.
I think instance fields should never be anything but private. Expose
them with getters.
I also think that System.out/err should never appear in the code, nor
printStackTrace(), with the possible exception of main() methods. Log
exceptions and messages with an SLF4J logger.
We should standardize on SLF4J for logging. I see direct use of
commons logging creeping back in.
toArray() should ideally be called with a right-sized array:
List<String> stuff = ...;
String[] stuffArray = stuff.toArray(new String[stuff.size()]);
Passing a 0-length array works in that it will be discarded and a new
array allocated, but that means the allocation was pointless.
'transient' is rarely needed, and never does anything in a
non-Serializable class.
These are redundancies that can be simplified:
if (someBoolean == true)
to
if (someBoolean)
int value = foo();
return value;
to
return foo();
int foo = 0;
foo = somethingElse();
to
int foo = somethingElse();
