レビューで鍛えるJavaコーディング力その１（日付バリデーション）

2009-12-24

■[Javaレビュー]レビューで鍛えるJava コーディング力その１（日付バリデーション）

現場でJavaのソースをレビューしていると、一目で「問題だ」と気づくコードによく出会います。

しかも、同じような問題をアチコチで見かけることも、少なくありません。

FindBugsを導入したおかげで、そういう問題が多少減ったものの、

やはりゼロになるわけではありません。

、、、ということで、

そんな問題コードに即座に反応できるようになるために、

私が見てきた問題コードをクイズ形式で紹介していきます。

ぜひ皆さんも、脊髄反射でNG部分を見つけてください。

見つけられなければ、適当にブクマでもしてください（笑

では今回は、日付チェックに関する問題です。

問題

以下のコードの問題を指摘し、修正してください。

ただし、問題は複数あることもあれば、全くないこともあります。

/**
 * 日付が正しい形式であり、存在する日付であることを検証します。
 *
 * @param date 日付を示す文字列
 * @return 正しい日付の場合はtrue、そうでない場合はfalseを返します。
 */
private static final DateFormat DF = new SimpleDateFormat("yyyy/MM/dd hh:mm:ss");

public static boolean checkDate(String date)
{
	try
	{
		DF.parse(date);
	}
	catch (ParseException ex)
	{
		return false;
	}

	return true;
}

シンプルな日付のバリデーションメソッドです。

呼び出し元では、この値を使ってCSV出力などを行うという想定です。

解答１

真っ先に思いつくのは

「SimpleDateFormatはスレッドセーフじゃないから、クラス変数として持っちゃダメ」

「setLenientしていないから、暦のチェックがされない」

の、どちらかではなかったでしょうか。

SimpleDateFormatが日付のパースやフォーマットを行うクラスということしか知らず、

どこかで見たサンプルを丸写ししているレベルだと、この問題に引っかかります。

SimpleDateFormatは、複数スレッドから同時にアクセスされてしまうと、

formatした結果がおかしくなったり、例外が発生することがあります。

なので、毎回SimpleDateFormatのインスタンスを生成するか、

Jakarta Commons LangのFastDateFormatクラスを利用しましょう。

また、DateFormat#setLenient(false)を指定していない場合

「2009/12/32 10:89:50」のような存在しない日付もパースし、

「2010/01/01 11:29:50」として扱ってしまいます。

目的がバリデーションである以上は、

「2009/12/32 10:89:50」はエラーとすべきでしょう。

だから、DateFormatの宣言部分は、以下のように修正すべきです。

public static boolean checkDate(String date)
{
	DateFormat df = new SimpleDateFormat("yyyy/MM/dd hh:mm:ss");
	df.setLenient(false);

	try
	{
		df.parse(date);
	}
	// 後は同じ
}

楽勝でしたか？

解答２

もう一つ、大きな間違いがあります。

それは "yyyy/MM/dd hh:mm:ss" です。

hhは時間を示す表示ですが、AM/PMがつくことを前提とした「12時間表記」なのです。

これでは「23:00:00」をエラーとみなしてしまうか、「11:00:00」と扱ってしまいます。

24時間表記は「hh」ではなく「HH」なので

"yyyy/MM/dd hh:mm:ss" ではなく "yyyy/MM/dd HH:mm:ss" と記述する必要があるのです。

かなりうっかり度が高いミスですが、

Javaを始めて1～2年経って、日付フォーマットやAPIを大体覚えてしまった人や、

特に問題コードを見てスレッドセーフやsetLenientの問題に脊髄反射した人ほど、

このミスを見落としやすかったのではないでしょうか。

解答３

まだ問題があります。

それは、SimpleDateFormat付フォーマットが前方一致であるため、

後ろに余計な文字列がついていても、パースできてしまう、という問題です。

たとえば "2010/01/01 11:29:50ですよ" とか

あるいは "2010/01/01 11:29:5a" なども、パースできてしまいます。

特に後者はJakartaのCommons Validatorを使っていても

チェックができなかったりするので、注意が必要です。

「入力は寛容に、出力は厳格に」の原則に従えば

別にパースできてしまっても良いじゃないか、という想いもありますが、

これはCSV出力するためのフォーマットチェックと位置づけていますから

厳密にチェックする必要があるでしょう。

要するにDateFormatで行うのは「暦の妥当性」チェックであって、

形式チェックはきちんと別に行う必要がある、ということです。

正規表現で形式チェックを行ったり、

長さチェックとsubstringとparseIntでのチェックを行なっても良いでしょう。

私は時々妥当性チェックとして、

「パースとフォーマットを行って、元の文字列を取得できるか」を確認する、

という方法を使うことがあります。

こんな感じです。

public static boolean checkDate(String date) {
	DateFormat df = new SimpleDateFormat("yyyy/MM/dd hh:mm:ss");
	String converted = null;
	try
	{
		Date convertedDate = df.parse(date);
		converted = df.format(convertedDate);
	} catch (ParseException ex) {
		return false;
	}

	return converted.equals(date);
}

パフォーマンスには疑問があるかも知れませんが、

setLenientの存在や、SimpleDateFormatが前方一致であることを知らなくても

この方法であれば、確実に日付をチェックすることができます。

この「変換と逆変換を行なって元の値を取得できるか」という可逆性チェックは

APIの細かい仕様を知らなくても、チェックできる、

つまり「安全性」というメリットがあるのです。