Javaの7大禁じ手 - Kengo's blog

「Javaの7大禁じ手」というブログタイトルを思いついたので、なんとかして7つダメコードをでっちあげたいless than a minute ago via Twitter for iPhone Favorite Retweet ReplyKengo TODA
eller86

などと書き込んだところいくつかネタ提供を頂きましたので、自分のネタと合わせてまとめてみました。Thanks to @flyaway !

Stringインスタンス同士を==で比較

Javaプログラムを始めた人がまず引っかかるのがこれでしょう。厄介なのは

System.out.println("abc" == "abc"); // => true
System.out.println("a" + "bc" == "abc"); // => true

といったコードが期待通りに動作してしまうということです。

System.out.println("abc" == new String("abc")); // => false
System.out.println("a".concat("bc") == "abc"); // => false
System.out.println(
	new StringBuilder("a").append("bc").toString()
	 == new String("abc")
); // => false

のように、ある程度複雑なコードを書かないと気づけません。でも最初はこういうコード書かないですよね。

浮動小数点同士を==で比較

Stringを==で比較すべきでないということは大抵の入門書に記載されていますが、こちらは意外と軽視されていることが多いかもしれません。浮動小数点もString同様、大抵の場合は問題なく動作します。

System.out.println(0.0 == 0.0); // => true
System.out.println(1.0 / 4 == 0.25); // => true

しかし==では、NaN同士の比較や+0.0と-0.0の比較が期待通りに動作しません。ラッパークラスに用意されている比較メソッドを使用する必要があります。

double plusZero = 1 / Double.POSITIVE_INFINITY;
double minusZero = 1 / Double.NEGATIVE_INFINITY;
System.out.println(plusZero); // => 0.0
System.out.println(minusZero); // => -0.0
System.out.println(plusZero == minusZero); // => true
System.out.println(Double.compare(plusZero, minusZero) == 0);  // => false

System.out.println(0.0d / 0.0); // => NaN
System.out.println(0.0d / 0.0 == Double.NaN); // => false
System.out.println(Double.compare(0.0d / 0.0, Double.NaN) == 0); // => true

ループと+=による文字列連結

特定の長さの文字列が欲しくなって以下のようなコードを書いたとします。

String text = "";
for (int i = 0; i < 1024; ++i) {
	text += "*";
}

一見簡潔なコードです。しかしJavaにおける文字列の連結はStringBuilder（Java 1.4以前はStringBuffer）クラスにて実現されますので、これは以下のコードと等価です。

String text = "";
for (int i = 0; i < 1024; ++i) {
	text = new StringBuilder(text).append("*").toString();
}

このため、1024個のStringBuilderインスタンスと1024個のStringインスタンスを使い捨てることになります。Javaヒープの総使用量は1MiBを超えるでしょう。

※ Javaヒープ総使用量の概算方法
StringBuilder内部のcharの容量 = （16＋17＋……＋1039）* 2 = 540,160 B
String内部のcharの容量 = （1＋2＋……＋1024）* 2 = 524,800 B
540,160 + 524,800 = 1,064,960 B ≒ 1.02 MiB
実際にはStringやStringBuilder自体の容量や、char[]のヘッダ情報の容量も消費する

今回のような文字列生成を最も高速に実現するのは、おそらく以下のコードです。

StringBuilder textBuilder = new StringBuilder(1024);
for (int i = 0; i < 1024; ++i) {
	textBuilder.append('*');
}
String text = textBuilder.toString();

ひとつのStringBuilderインスタンスを最後まで使うことによりStringとStringBuilderの無駄なインスタンス生成を省くとともに、必要な容量をあらかじめコンストラクタで明示することによりStringBuilder内部のchar[]の自動拡張を回避しています。また若干ですがStringBuilder#append(String)よりもStringBuilder#append(char)の方が高速であることが期待されます。

main以外でのSystem.exit(int)

プログラムが返すステータスコードを決定するために使用する必要があるSystem#exit(int)（あるいはRuntime#exit(int)）ですが、他のメソッドと異なり呼び出し元コードに復帰しないという特徴を持っています。
このためプログラムの深い部分でこれを呼び出すと、実行を期待されていたメソッドやfinallyブロックを実行しないまま終了してしまいます。

個人的にはC言語と同様に、このメソッドを無くしてmain関数の戻り値をint型にした方が無難だったと思うのですが……。歴史的経緯などご存じの方がいらっしゃいましたらお聞かせいただきたいです*1。

SimpleDateFormatなどを複数のスレッドから使用する

Java標準APIにはスレッドセーフでないクラスが多数存在します。SimpleDateFormatクラスなどはよく知られた例でしょう。synchronizedブロックやjava.util.concurrent.locksパッケージのクラスによって、同一インスタンスを同時に使用しないといった回避策を取る必要が出てきます。
もっともローカル変数として使う分には心配する必要はありませんし、大抵の場合はスレッドセーフな代替手段が存在します。SimpleDateFormatクラスの代替手段にはFastDateFormatクラスがApache Commons Langから提供されています。

finalize

明示的にJavaヒープをfreeできないJavaにおいて、finalizeは救世主のように見えるかもしれません。これさえ使えば、インスタンスが要らなくなったタイミングでリソースを確実に開放してくれそうです。が、そんなことはありません。
finalizeがダメな理由や有効なケースについてはジョシュア・ブロック氏が5ページに渡って語ってくださっていますので、そちらを参照願います。いや、ほんとにいい本ですよEffective Javaは。

Effective Java 第2版 (The Java Series)

作者: Joshua Bloch,柴田芳樹
出版社/メーカー: ピアソンエデュケーション
発売日: 2008/11/27
メディア: 単行本（ソフトカバー）
購入: 62人クリック: 781回
この商品を含むブログ (250件) を見る

ひとつ関連する話を紹介すると、finalize()を実装したクラスのインスタンスは参照されなくなってから初めて実行されたGCでは解放されず、その後finalize()が実行されて初めて開放対象となります。GCを実行してもJavaヒープが解放されないため、たとえ既に参照されていなくてもOutOfMemoryの引き金になりえます。
finalize()を実装したクラスのインスタンスだけでなくそのインスタンスが参照するオブジェクトもまた初回のGCでは解放されません。そのためどうしてもfinalize()に頼らなければならない場合は、インスタンスが参照するオブジェクトの総量が大きくなりすぎないように注意すると良いでしょう。

文字数をString#length()で数える

Stringに含まれる文字数を数えるためにString#length()を使用しているケースは多いのではないでしょうか。しかしこのメソッドは、サロゲートペアを含む文字列においては期待通りに動作しません。このメソッドは文字列内のUnicodeコード単位の数を返すメソッドに過ぎないためです。
私がコミッタとして開発に参加させていただいているPARTAKEでは、String#codePointCount(int, int)を使ったユーティリティメソッドを用意することでこの問題に対応しています。

public static int codePointCount(String s) {
	return s.codePointCount(0, s.length());
}

選外

禁じ手と呼ぶほどでもないがやっぱりダメなコードや、気づかないと分かりにくい問題など。Javaの落とし穴とでも呼ぶべきでしょうか。

Vector, Stack, Hashtable, StringBufferなどレガシー実装

今から新規にJavaを書くならば、これらのクラスのお世話になるべきではないでしょう。それぞれArrayList, LinkedList, HashMap, StringBuilderといった効率のよい実装が存在します。
ただHashtableはPropertiesクラスの親クラスとして、今後のプログラムにもしぶとく生き残っていくかもしれません。

new Integer(int)などラッパークラスのコンストラクタ

Integer.valueOf(int)などといったファクトリメソッドが存在しますので、こちらを使用すべきです。プリミティブ型のラッパークラスはすべてImmutableですので、実行環境がうまくインスタンスを使いまわしてくれるかもしれません。

Javaヒープの連続領域を確保するクラス

Javaの配列はJavaヒープ上に連続領域として確保されます。配列生成時に連続領域が確保できないとOutOfMemoryErrorが発生しますので、数MiB〜数十MiBを必要とする配列の生成は避けるべきでしょう。
例えばHTMLやJSONのような長めの文字列を生成する場合は、メモリに溜めてから書き出すのではなく逐次Writerに書き出す実装を選択することを検討すべきです。

配列を使用する有名なクラスには、ByteArrayOutputStream, BufferedInputStreamなどのBuffered〜系, ArrayList, String, StringBuilderなどがあります。

引数に正規表現を受け取るメソッド

Java標準APIには、正規表現をPatternではなくStringとして受け取るメソッドがいくつか存在します。String#split(String)がその一例です。ですからバックスラッシュで分割する場合は、"\\"ではなく"\\\\"を渡さなければなりません。Stringを引数に取るメソッドを使う場合はJavadocに注意しましょう。

String path = "\\dir\\file.txt";
// String[] splitPath = path.split("\\"); // => throws PatternSyntaxException
String[] splitPath = path.split("\\\\"); // => OK
String fileName = splitPath[splitPath.length - 1];

（追記）MapのkeyにUncomparableなクラスのインスタンスを使う

@2t3氏より1つご紹介いただきました。

@eller86 MapのkeyにUncomparableなもの(ex.byte[])ってのはどーでしょ。less than a minute ago via Tween Favorite Retweet ReplyHayato Tsutsumi
2t3

例えばHashMapやTreeMapにbyteをキーとして突っ込み、その後同じ中身を持つ別のbyteでgetしてみます。

for (Map<Object, Object> map : new Map[]{
	new HashMap<Object, Object>(), new TreeMap<Object, Object>()
}) {
	map.put(new byte[]{0}, "we cannot get this value");
	System.out.printf("when getting by key(new byte[]{0}) from %s, returned value is %s%n%n", map.getClass().getName(), map.get(new byte[]{0}));
}

すると、HashMapの場合はnullが返ってきますし、TreeMapの場合はput時に例外が発生してしまいます。

when getting by key(new byte[]{0}) from java.util.HashMap, returned value is null
Exception in thread "main" java.lang.ClassCastException: [B cannot be cast to java.lang.Comparable
at java.util.TreeMap.getEntry(TreeMap.java:325)
at java.util.TreeMap.get(TreeMap.java:255)
at in.partake.tool.EntityListUper.main(EntityListUper.java:11)

*1:本記事の公開後、@koichik氏より[https://twitter.com/#!/koichik/status/104779852699287553:title=マルチスレッド対応という視点]についてご教示いただきました。ありがとうございます。