やっぱ、仕事でJavaやる人はEffective Javaは読んでおくべきだと思うよ。

めずらしく仕事の話なのですが、なんか年明けから他部署に出稼ぎに行かされています。で、その仕事の内容というのが「別の誰かがつくった膨大なJavaソースにJavadocを書き込む」という訳の分からないことをやらされています。しかも、そのJavadocというのが普通のクラスやメソッドの外部仕様について書くのではなくて、完全な内部仕様でほとんどソースの和訳みたいなのを書かなきゃいけないという･･･。年頭からいったいどうやってやる気を奮い立たせればいいのか分からなくなってきます。まったく。
まあ、とにかくソースを読んでがんばってJavadocを書いてるわけなのですが、人のコードを見るとどうもアラに目がいってしまいエントリのタイトルの通りに思うわけですよ。ハラに溜めておくのは精神衛生上よくないと思うので、気づいたのをここに列挙してみます。

列挙する

nullでないことが確定されている変数をnullチェックするな。
- nullを返さないことが仕様として決まっているメソッドの戻り値をnullチェックするな。
  - Class.forNameはnullを返さない。org.w3c.dom.Element#getChildNodesはnullを返さない。StringBuffer#toStringはnullを返さない。お前のそのメソッドはnullを返さない。nullチェックするな！
- キャストしただけなのにnullチェックするな！
- nullでないことを確定させるために変更されないフィールドはfinalにしろ。
処理失敗時に安易にnullを返すな。
- nullが返却されるパターンはjavadocにすべて記載しろ。
- 引数チェックエラー時にnullを返すな。IllegalArgumentExceptionやNullPointerExceptionなどを投げろ。
catch(RuntimeException e)するな。catch(Exception e)するな。
絶対に参照されることがない値で変数を初期化するな。
配列型のフィールドをpublicにするな。
- ミュータブルなフィールドをメソッドから返却して変更させるな。
  - コレクションや配列はコピーか変更不可なラッパーを返せ。
多数の文字列との比較はelse ifの連続ではなく、ループで比較せよ。
変数の頭にstrとかobjとかつけるな。頭悪い！
実装クラス型ではなくインタフェース型を使え。
ただの数を定数定義するな！
一回バイナリサーチするためだけに配列をソートするな。
- 当たり前だが、ソートのオーダーはO(n*logn)だから線形検索のO(n)より重い。
変数名で嘘をつくな。
ローカル変数を使いまわすな。
引数のnullチェックでチェック例外を投げるな！
出力引数を使ったvoidメソッドにgetHogehogeと名づけるな。

Effective Javaと全然関係ないのも多いですが。

少しだけ解説。

nullでないことが確定されている変数をnullチェックするな。
なぜかこのプロジェクトのソースは無駄なnullチェックがやたら多くて、ほとんどの受け取ったメソッドの引数や呼び出したメソッドの戻り値に対してnullじゃないかチェックしている。Class#forNameやStringBuffer#toStringがnullを返すなんてことは絶対にありえないのに。しまいには変数をキャストしただけなのにnullになっていないか確認しだす始末。その結果、絶対に実行されないエラー処理があちこちに･･･。
処理失敗時に安易にnullを返すな。
上の「nullチェックするな！」にもかかるのですが、Javaにおいて処理の失敗は例外を使ってあらわすべきことです。通常の戻り値は通常の処理としてあるべきで、処理に失敗した（DBに接続できなかったとか）ときにnullを返すのは悪い習慣です。こんなことがまかり通っているからプログラマがnull恐怖症に陥って、ありもしないnullにおびえて要りもしないnullチェックを連発するようになるのでしょう･･･。
実際にjavaの標準ライブラリーで処理失敗を表すためにnullを返すことなんて見たこと無いです。
絶対に参照されることがない値で変数を初期化するな。
Javaでは初期化されてない場合のあるローカル変数にアクセスしようとするとコンパイルエラーになります。コンパイラがCなどでありがちなバグのパターンを事前にチェックして見つけてくれるわけです。それなのに変数宣言時に意味なくnullで初期化してしまったりするとこの機能がほとんど意味なしになってしまいます。意味の無い値でローカル変数を初期化しないでください。
変数の頭にstrとかobjとかつけるな。
いわゆるシステムハンガリアンなんですが、固有接頭辞があるのがint型(n)、boolean型(b)、String型(str)、HashMap型(hm)、List型(lst)しかなくて、他のObjectはみんな"obj"。そんなんじゃ結局ほとんど"obj"じゃないか！と思ったら圧倒的に多いのは"str"だったり。それはそれでとても「不吉な匂い」なんだけど。しかし、なんでこんなクソの役にも立たないものも自然に淘汰されてなくなってくれないのでしょう？「オレは（なにか）やっているぜ！」という安心感でも与えるのでしょうか？
ただの数を定数定義するな！
```
/** ナンバー16. */
public static final int NUMBER_ONE_SIX = 16;
```
本当にこんなコードを書く人がいるとは正直驚きました。実際にはparseIntの基数（16進)を指定するのに使われていました。他にも意味深な数字がいっぱい定義されています。リテラル直書きをこんな定数定義にしたからって、意味が示されてなきゃマジックナンバーであることに変わりがないです。