Ah, we can all put in our coding pet peeves... :-)
On Aug 22, 2008, at 10:20 AM, Sean Owen wrote:
OK here's a lot more of my notes as I'm run analysis on the code:
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");
Definitely.
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()
+1
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.
+1
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.
+1
We should standardize on SLF4J for logging. I see direct use of
commons logging creeping back in.
+1. Didn't catch that
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)
I disagree. I am more than likely the guilty party here and I'll own
up to it and even take credit for it. I don't know about you, but at
2 in the morning after staring at the same code for hours trying to
find that stupid bug that I just can't track down, I find
boolean lNamedVariable = ...
if (!lNamedVariable)
to be much harder to read than:
if (lNamedVariable == false)
which then makes it easier for me to understand exactly what the
condition is and better decide if that is truly the intent.
and the same goes for == true. I know _A LOT_ of people disagree with
that, but for me, I've seen that one bite many people b/c they gloss
over reading it, especially when the name of the variable starts with
an "l" or some other character that has a look similar to the
exclamation point, or their font size is small or their eyes are tired
or whatever.
Just my two cents,
Grant
