S-JIS[2008-04-16/2014-10-04] 変更履歴

Javaのダメなコーディング

Javaのアンチパターンと言うほど高尚なものではないけれど、「こんな事しちゃアカンよ」という例。

• Formatを同期せず使用 [/2008-05-12]
• 不要なnew [2008-04-26]
• Stringの等値比較方法
• 定数==変数 [/2008-05-23]
• Integerの使用方法 [/2008-05-21]
• Exceptionの使用目的
• 回数指定ループ [2008-04-26]
• イテレーター＋カウントパターン [/2014-10-04]

Format系クラスの同期

Format系のクラスはMTセーフでない。[2008-04-26]

Format系のクラスというのは、java.text.Formatを継承したクラス、すなわちDateFormatとかNumberFormat（DecimalFormat）といったクラスのこと。
これらのクラスのJavadocには、「同期」という項で「同期化されません」（すなわちスレッドセーフでない）と明記されている。

したがって、スレッドを使っていないバッチ系のプログラムならともかく、他の場合でstatic変数でFormat系のインスタンスを保持するのは危ない。
（Web系（サーブレット・JSP・Struts）は、staticでないフィールドでもアウト）

• スレッドを使っているバッチ
• サーブレット
• JSP（特にJSP宣言）
• StrutsのAction

	private static final DateFormat df = new SimpleDateFormat("yyyy/MM/dd");

		Date date = df.parse("2008/04/26");

一見、大丈夫そうに見えてしまうのが怖い。（Format系クラスの設計ミスという気がしないでもないが…）

→SimpleDateFormatの同期のさせ方 [2008-05-12]

不要なインスタンス生成

変数の初期化で無駄なインスタンスを生成する例が見られる。[2008-04-26]

	SampleBean bean = new SampleBean();
	bean = OtherClass.getSampleBean();

変数初期化の直後に別のメソッドから値を代入し直しているので、最初にnewで生成したインスタンスが全くの無駄になる。（生成にコスト（時間とメモリー）が使われるので無駄、GCでの回収作業も発生するのでさらに無駄）

コンストラクターの中に副作用（例えば共通クラスに自分自身を登録するとか）があればnewするのも意味があるが、逆にそういう仕様（外部から分かりにくい仕組み）にはしない方がいいと思う。

• Stringでこういった初期化をするのも同じ無駄
• どちらかというとローカル変数は初期値を与えない方がいいと思う

Stringの等値比較

文字列同士の等値比較に演算子==を使うべきではない。

	String s = "abc";
×	if (s == "abc") 〜
○	if (s != null && s.equals("abc")) 〜
□	if ("abc".equals(s)) 〜

	String s = new String("abc");
	if (s == "abc") 〜		これが不一致になる

→intern()を使えば一致するが…

「必ず決められた定数しか使わない」と決められていて常にそれに従っているなら==で比較しても動作上の問題は無いが…
JavaでStringの等値比較に==を使わないのは最早ほぼ常識なので、普通のJavaプログラマーが見た場合に誤解を招く可能性が高い。

	final String STAT1 = "abc", STAT2 = "def";
	String s = STAT1;
	if (s == STAT1) 〜

こういう使い方ならば、Stringじゃなくてintでも構わないでしょう（というより、if文だけ見たらintみたいに見えてしまう）。
JDK1.5以降なら、そもそも列挙（enum）を使うようなケース。

定数==変数

変数と定数が等しいかどうかを比べる場合、変数を左に書くべき。[2008-05-02]
プログラミング言語は仕様（自然言語）の書き順と合わせた方が読みやすいので、自然な順番を目指すと 変数は「==」の左側になる。

	int 変数 = …;
○	if (変数 == 123) 〜
×	if (123  == 変数) 〜

定数を左に書く方法は、C言語の場合には意味がある。
しかしJavaの場合は、if文等の条件を書く部分は必ずboolean型でなければならないと決まっている。
したがって間違って「=」1つで代入式になってしまったとしても、その変数がbooleanでない限り（C言語とは違って）コンパイルエラーになる。
だからJavaではこのような技巧に走る必要は無い。

	int 変数 = …;
	if (変数 = 123) 〜		…「intからbooleanに変換できない」というコンパイルエラーになってくれる

ではbooleanの場合はどうかというと、確かにエラーになってくれない。

	boolean 変数 = …;
	if (変数 = true) 〜	…常に真になってしまう（コンパイルエラーにならず、実行時に発覚する）
	if (true = 変数) 〜	…「代入の左側が変数でない」というコンパイルエラーになってくれる

（Eclipseを使っているなら、if文の条件の中のbooleanの代入を エラーにすることも出来る。[2008-05-23]）

しかし、そもそもbooleanはtrueやfalseと比較すべきでない（==や!=を使うべきではない）。
boolean変数（やメソッド）は以下のようにして使う。

	boolean 変数 = …;
	if (変数) 〜
	if (!変数) 〜
	if (判断メソッド()) 〜
	if (!判断メソッド()) 〜

（実際には、その真偽値の意味がパッと分かるような変数名・メソッド名を付けるべき↓）

	if (isDigit(…)) 〜
	if (!isDigit(…)) 〜

したがってJavaにおいては、条件判断に「==」を使う箇所で変数を右側に書く理由は無い。

なお、オブジェクトの等値比較（equals()）においては、変数を右側に書くのはもっともな理由がある。→Stringの例

Integer

プリミティブ型（intとか）とラッパークラス（Integerとか）と文字列の相互変換方法について。

×	int n = new Integer(str).intValue();		Integerインスタンスを生成する分ムダ
○	int n = Integer.parseInt(str);	

×	String s = new Integer(n).toString();	Integerインスタンスを生成する分ムダ
○	String s = Integer.toString(n);

JDK1.5以降では、以下のようなものも。

△	Integer i = new Integer(n);
○	Integer i = Integer.valueOf(n);		小さい数のときはIntegerインスタンスを使い回してくれる

ただし-128〜127以外の数を主に使うのであれば、new Integer(n)を直接使う方が効率が良い。

Integer等のラッパークラスは不変のオブジェクトなので、定数（固定値）ならば毎回newするのは無駄。

	bean.setValue1(new Integer(0));
	bean.setValue2(new Integer(0));
↓
	final Integer INT0 = new Integer(0);
	bean.setValue1(INT0);
	bean.setValue2(INT0);

特にBooleanでは true・falseに該当する定数が定義されているので、それを使うべき。[2008-05-21]

	new Boolean(true)	→ Boolean.TRUE
	new Boolean(false)	→ Boolean.FALSE
	new Boolean(変数)	→ Boolean.valueOf(変数)

正常系で例外を使用

例外（Exception・Throwable）は、あくまでも異常が起きた場合に使うものであり、正常処理の戻り値や分岐として使うべきではない。

×	try {
		if (条件1) {
			throw new 男();
		}
		if (条件2) {
			throw new 男();
		} else {
			throw new 女();
		}
	} catch (男 e) {
		//男性用処理
	} catch (女 e) {
		//女性用処理
	}

switch文・goto文の代わりってか。
Javaの文法としては間違ってはいないが、Javaが出始めた初期の頃ならいざ知らず、最近ではさすがにこんなコーディングは聞いたことも無い。

RDBの場合、SELECT・UPDATE・DELETE等のDMLの結果がWHERE条件によって0件になる場合があり、これ自体はDBとしては正常な動作である。
JDBCを使ったアプリケーションとしては、あるテーブルの操作結果が必ず0件以外（対象データが存在する）のはず、という仕様もあるだろう。この場合、0件だったら例外を発生させるというのは頷ける。
しかし0件であっても正常、という処理も当然ある。

なので、DBアクセスのフレームワークが結果が0件だったら必ず例外を発生させるような造りになっていたとしたら、それはおかしい。
実装上はそういう例外をキャッチして握りつぶせばいいのだが、例外の発生というのはそれなりにコストの高い処理なので、フレームワークという中核部分の設計・コーディング思想がそんなでは困る。

回数指定ループ

回数を指定したforループのコーディング方法について。[2008-04-26]

例えば10回回すのは以下の2通り考えられる。

	for (int i = 0; i < 10; i++) 〜

	for (int i = 0; i <= 9; i++) 〜

どちらも結果は同じだが、見た目、すなわち考え方が異なる。
この例での“仕様”は「10回」なので、「10」という数値が出てくる前者が素直で良い。後者はひねくれている。
　

	for (int i = 1; i <= 10; i++) 〜

インデックスが1から始まって10回なら、上記のようになる。しかし通常は、Javaでは0から始まるインデックスを使うことが多い。
（BASICでは、一般には1から始める。「FOR I = 1 TO 10」）

イテレーター＋カウント

Iteratorを使ってデータを取得しつつ、何件目かを知りたい場合。[2008-05-02]

パターン１：

	Iterator<Data> it = list.iterator();
	int size = list.size();
	for (int i = 0; i < size; i++) {
		Data data = it.next();
		System.out.printf("[%d]:%s\n", i, data.toString());
	}

Iteratorを使っているのにイテレーターパターンでないのはどうかと思う。

パターン２：
という訳で、以下のようにするのがいいかな。

	int i = 0;
	for (Iterator<Data> it = list.iterator(); it.hasNext(); i++) {
		Data data = it.next();
		System.out.printf("[%d]:%s\n", i, data.toString());
	}

パターン３：
これなら、素直にfor-each構文に変換することも可能。

	int i = 0;
	for (Data data : list) {
		System.out.printf("[%d]:%s\n", i, data.toString());
		i++;
	}

ただし、途中でcontinueを使う場合はループ末尾のi++が実行されないことになるので注意。

パターン４：
なお、ArrayListであれば以下のようにするのが最も高速。

	int size = list.size();
	for (int i = 0; i < size; i++) {
		Data data = list.get(i);
		System.out.printf("[%d]:%s\n", i, data.toString());
	}

パターン５：
sizeというローカル変数を無くしてみた（ループの終了条件で毎回List#size()を呼び出す）。

	for (int i = 0; i < list.size(); i++) {
		Data data = list.get(i);
		System.out.printf("[%d]:%s\n", i, data.toString());
	}

パターン６：

ListIteratorを使ったパターン。[2014-10-04]
変数が少なくてすっきりする。

	for (ListIterator<Data> it = list.listIterator(); it.hasNext();) {
		int i = it.nextIndex();
		Data data = it.next();
		System.out.printf("[%d]:%s\n", i, data.toString());
	}

パターン７：

JDK1.8で追加されたforEachメソッドを使ったパターン。[2014-10-04]
ループの外に変数を持たせる方法がいまいちorz

	int[] n = new int[1];
	list.forEach(data -> {
		int i = n[0]++;
		System.out.printf("[%d]:%s\n", i, data.toString());
	});

以上のパターンでListの実体を変えてループ内でDataを取得するだけ（System.out.printは実行せず）の処理にして実行時間を測ってみた。

うーむ、綺麗なIteratorパターン（hasNext()を使ったパターン２・３）は綺麗でないパターン１より遅いのか。困った話だ。（と言っても200万件で20ms、500万件でも80msの差だから、そんなに大きな問題じゃないけど）
（ちなみにLinkedListを500万件で試していないのは、OutOfMemoryErrorになったから。
　ArrayListよりもLinkedListの方がエントリー当たりのメモリー消費量が多い。→IBMのJava コードから Java ヒープまで [2012-04-13]）

WindowsXP・JDK1.6で測定したときは200万件ならLinkedListの方が速かったが、Windows7・JDK1.8ではArrayListの圧勝。