読者です 読者をやめる 読者になる 読者になる

ソースコードにWhy(なぜ)を残していますか?

雑記

つい最近Java界隈でSimpleDateFormatがスレッドセーフではない、という事が話題になりました。JavaDocにも「synchronizedではない」と書かれている事だけれども、日付の変換は良く使う上にフォーマットを繰り返し書くのは面倒なので、ユーティリティクラスとしてまとめておく事が良くあるっしょ。
こんな感じで。

package util;

import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;

public abstract class DateConverter {
	
	public static String convertYYYYMMDD(Date date){
		DateFormat formatter = new SimpleDateFormat("yyyy/MM/dd");
		return formatter.format(date);
	}
	
}

で、何もしらない人がこのコードを見たとき、毎回SimpleDateFormatを作成するのは無駄じゃん!*1と思ってこんな感じにしたとしましょう。

public abstract class DateConverter {

	private static DateFormat YYYYMMDDFormat = new SimpleDateFormat("yyyy/MM/dd");
	
	public static String convertYYYYMMDD(Date date){
		return YYYYMMDDFormat.format(date);
	}
	
}

冒頭にも書きましたが、SimpleDateFormatで行う処理はsynchronizedされません。複数のスレッドから同時にこのクラスにアクセスされた場合、違う結果が返された、という障害が起りうるわけです。しかも再現が非常にムズい、修正者泣かせの障害でしょう。はい、わかりましたわかりました、元に戻します…。となったところで、戻した理由を書いていなかったら同じ過ちが繰り替えされる可能性は消えませんよね。少なくとも繰り替えさないようにコメントを残しておきましょう、と言うのがこの日記の趣旨です。こんな感じ

public abstract class DateConverter {
	
	public static String convertYYYYMMDD(Date date){
		// bugs:xxxx SimpleDateFormatはスレッドセーフではないため、毎回インスタンスを作成するようにしています。
		DateFormat formatter = new SimpleDateFormat("yyyy/MM/dd");
		return formatter.format(date);
	}
	
}

他にも、ユーティリティクラスについての議論で、今後も新しい人が入ってくる度にもめそうな事があれば、それについてコードに残しておく方がよさそうです。こんな感じ。

public class DateConverter {
	
	public static String convertYYYYMMDD(Date date){
		// bugs:xxxx SimpleDateFormatはスレッドセーフではないため、毎回インスタンスを作成するようにしています。
		DateFormat formatter = new SimpleDateFormat("yyyy/MM/dd");
		return formatter.format(date);
	}
	
	/*
	 * こんな感じでコンストラクタの非公開の理由も書くといいかもしれませんね。
	 * 
	 * このクラスはユーティリティクラスのため、コンストラクタを非公開にしています。
	 * クラス修飾子にabstractを用いる事を検討しましたが、
	 * チームで議論した結果、abstractが示す意図とここでの用法は違うという意見が出たため、
	 * コンストラクタを非公開にすることになりました。
	 * 
	 * @see 2009/03/21の議論 http://kompiro.org/....
	 */
	private DateConverter(){}

}

JavaDocにWhyを記述することも考えられますが、JavaDocに記述すべきなのはそのクラスの利用者が注意しなければならない用途に関してかと思ってます。例えばDateConverter#convertYYYYMMDD(Date)の引数がnullだった場合、NullPointerExceptionが投げられます。*2nullを渡さないよう、利用者が気をつける、というルールであれば、JavaDocに素の旨を書いておくべきです。*3JavaDocに残すべきなのは実装を意識した振る舞いではないはずです。
そういえばかくたにさんも以前「ソースコードにコメント以外Whyを残す手段がない」と言っていた気がするその後どうなったんだろう。

*1:プロファイルした結果問題になるくらい問題になってた、とします。そんなことほとんどないですけど。

*2:それはDateConverterクラス実装者の責務っぽいけど

*3:でもJavaDocに書いておいても読まずに実装する人が多いので、コードの中に残せない理由をコメントに残そう、というのが今回の趣旨ですから