JDK5でGenericsが採用されたので、
以前よりも配列を使うメリットは無くなりましたが
完全に使う機会を失ったわけではありません。
List<String> list = new ArrayList<String>(); String[] result = list.toArray(new String[list.size()]);
Genericsを使っていれば変換のコードも(キャストが不要な分)若干短くなります。
PMD : NullAssignment
FindBugs : DLS: Dead store of null to local variable
一昔前によく言われた事が、
「オブジェクトを使わなくなったら null を代入する」という不文律のようなものです。
これはGC(ガーベッジコレクト)の効率を高めるという意味があるようですが
それ以前にまず、ロジックを見直した方が良いというのが最近の考え方です。
そもそも、オブジェクトが使われなくなっても
まだそのオブジェクトがスコープから外れないという状況が良くないのです。
private Resource resource;
public void bar() {
resource = new Resource();
...
resource = null;
}
このような場合、resourceはローカル変数で定義してやれば良いのです。
public void bar() {
Resource resource = new Resource();
...
// resource = null;
}
こうすれば、bar() メソッドが終了するときにresourceはスコープから外れるので
nullを代入する必要がありません。
余談ですが、Java SE 6.0 ではローカル変数にnullを代入しても
GCの効率は変わらなくなるそうです。
sun. で始まるパッケージ等は、環境によって
あったり無かったりします。
クロスプラットフォームを実現する為には、これらのパッケージを
明示的にimportしてはいけません。
これらのパッケージは内部的に使用されるものです。
PMD : DontImportSun
通常、あるクラスというのは他のクラスに依存しています。
しかし、この依存関係があまり大きくなり過ぎる事は良くありません。
例えば、そのクラスを他のプロジェクトで使おうと思った場合に
その依存するクラス全てを持ってこなければなりません。
他にも、依存性が高いと他クラスが変更された場合の影響度が高くなり
バグが混入しやすくなります。
ある調査では、コードの保守コストは
そのクラスが依存するクラス数の2乗に比例するという結果があるそうです。
PMD : CouplingBetweenObjects / ExcessiveImports
CheckStyle : ClassDataAbstractionCoupling / ClassFanOutComplexity
多くのクラスは、いくつかのインターフェイスを実装しています。
例えば、ArrayListクラスはSerializable, Cloneable, Iterable, Collection, List, RandomAccess
というインターフェイス群を持っています。
こういったクラスのインスタンスを生成するとき、その受け皿は
なるべく「上位のインターフェイス」を使うことが好ましいです。
例を挙げると、
ArrayList list = new ArrayList();
とするのではなく
List list = new ArrayList(); Collection collection = new ArrayList();
のようにします。
なぜかというと、その後のコード修正がしやすくなるからです。
例えば、ArrayList を LinkedList に変更するような修正が入ったとします。
この場合、先程示した3つのコードは以下のように書き換えられます。
LinkedList list = new LinkedList(); List list = new LinkedList(); Collection collection = new LinkedList();
最初のコードでは2箇所の修正が必要なのに対し
後のコードでは1箇所しか修正の必要がありません。
たった1箇所修正する位だったら大した差は無いかと思うかもしれませんが、
目に見えないところではそれ以上に大きな差があります。
先程話した「依存性」の問題です。
ArrayList list = new ArrayList();
↑このようにすると、この処理ロジックは ArrayList だけでなく
それが持つ全ての基底クラスおよびインターフェイスに(内部的に)依存することになります。
Collection collection = new ArrayList();
↑このようにすれば、この処理ロジックは Collection およびこの基底インターフェイス、
それと ArrayList にのみ依存することになります。
具体的な数で言うと、前者ならば10、後者ならば3の依存数になります。
どちらが保守性に優れているかというのは言うまでもありません。
PMD : LooseCoupling
switch文には、default節を付ける事が出来ます。
これは付ける事を必須とするように心掛けましょう。
switch (v) {
case 'A':
// Aの処理
break;
case 'B':
// Bの処理
break;
}
変数cの状態に応じて、それに合わせた処理をしています。
この段階では、変数vは A / B のどちらかしか取らない事が前提となっています。
しかし、その後の修正で C という値を取るようになったらどうなるでしょう。
default節が無いと、変数vが C のときにこのコードは何もしません。
本来ならCの処理をしなければいけないのですが、それに気付く事が出来ないのです。
switch (v) {
...
default:
throw new IllegalStateException("vの値が不適切です");
}
このようにdefault節を記述しておけば、C という値が入ってきたときに
例外が返されるので、ここにCの処理を追加しなければならないという事に
気付くことが出来ます。
PMD : SwitchStmtsShouldHaveDefault
CheckStyle : MissingSwitchDefault
コンストラクタ内での処理は多くのトラブルの原因になることがあります。
スレッドを開始したり、オーバーライドされたメソッドを呼び出したりする事は
誤動作の原因になることがあります。
一般的に、コンストラクタ内での処理は必要最小限にするように心掛けましょう。
PMD : ConstructorCallsOverridableMethod
Singletonクラスというのは、インスタンスを一つしか持たないクラスの事です。
そういう意味ではユーティリティクラスと似ています。
Singletonクラスは、スレッドセーフに実装しなければなりません。
PMD : NonThreadSafeSingleton
変数名の命名規約については、色々と好みが分かれるようです。
せめてプロジェクト単位での統一はするようにしましょう。
そうしないと、他の人が保守しにくくなります。
PMD : VariableNamingConventions
ユーティリティクラスなど、拡張する可能性の無いクラスに関しては
明示的にクラスに final 識別子を付けましょう。
そして private のコンストラクタを定義します。
こうすることによって、
new Utils().bar();
このような間違ったコーディングをされる事を防ぐことが出来ます。
CheckStyle : FinalClass / HideUtilityClassConstructor
よく、こんなコーディングを見かけます。
public class Foo implements IConstants {
}
これは、定数を IConstants インターフェイスに定義しておいて
実装することでクラス内からそれらの定数を利用するという考えなのですが
これは正しいインターフェイスの使い方とは言えません。
まぁ便利なのはわかりますけど。
JDK5からは、import static という構文が追加されています。
例えば、
int result = IConstants.OK;
ように記述していた箇所を
import static IConstants.OK; ... int result = OK;
このように記述できます。
いずれ各種IDEも自動でこのような対応をしてくれるはずですので
定数インターフェイスを使うのはなるべく止めるようにしましょう。
CheckStyle : InterfaceIsType
PMD : AvoidConstantsInterface
最大のアンチパターンと言ってもいい、重複コードについてです。
重複コードは、出来る限りまとめましょう。
それが、プロジェクトを肥大化させない最良の方法です。
ちなみに、重複コードをチェックする Simian というプロジェクトがありますが
フリーライセンスではないので僕は使ったことがありません。
Ant や Maven など、質の高いライブラリでも使われているようなので
使ってみる価値はありそうです。
CheckStyle : StrictDuplicateCode
可能な限り、絶対パスは使わないようにしましょう。
基準パスからの相対パスで記述するだけで良いのです。
FindBugs : DMI: Code contains a hard coded reference to an absolute pathname
戻り値として配列を返すメソッドがあります。
ここで戻り値として「空」を返したい場合、
nullを返すのではなく「サイズ0の配列」を返しましょう。
int[] getWidths() {
...
return new int[0]; // return null;とはしない
}
nullを使うと、その後 NullPointerException が発生する可能性があります。
それよりも、サイズ0の配列を返した方が安全だという訳です。
バグが出る可能性は出来る限り減らしましょう。
それが、質の高いアプリケーションを開発するための第一歩です。
FindBugs : PZLA: Consider returning a zero length array rather than null