谷本 心 in せろ部屋

はてなダイアリーから引っ越してきました

レビューで鍛えるJavaコーディング力 その2(バリデーション)

前回の問題は、そこそこ好評だったようで、嬉しい限りです。
これからも、週に1〜2回ぐらいのペースで問題を出していこうと思います。


では、今回の問題です。

問題

以下のコードの問題を指摘し、修正してください。
ただし、問題は複数あることもあれば、全くないこともあります。

public class EmpService {
	EmpDao empDao = new EmpDao();

	public void register(Emp emp) {
		validate(emp);
		empDao.insert(emp);
	}

	protected void validate(Emp emp) {
		// 他のチェックは割愛。
		// 例外処理や、メッセージ部分は、分かりやすさのためにベタ書きしているため指摘対象外。

		if (ValidatorUtils.checkRequired(emp.getName()) == false) {
			throw new ValidatorException("名前は必ず入力してください。");
		}

		if (ValidatorUtils.checkByteLength(emp.getName(), 20) == false) {
			throw new ValidatorException("名前は20文字以内で入力してください。");
		}
	}
}

public class ValidatorUtils {
	public static boolean checkRequired(String str) {
		if (str == null || str.length() == 0) {
			return false;
		}

		return true;
	}

	public static boolean checkByteLength(String str, int max) {
		return checkByteLength(str, 0, max);
	}

	public static boolean checkByteLength(String str, int min, int max) {
		if (str == null) {
			return true;
		}

		int length = str.getBytes().length;
		if (min <= length && length <= max) {
			return true;
		}

		return false;
	}
}

いわゆるありがちなサービスクラスと、バリデーションクラスです。
データのバリデートを行なった後、DBへの登録を行います。

前回の振り返り

解答が見えてしまって残念な気持ちにならないよう、
ちょっと前回の振り返りでも挟みましょうか。


前回の日付チェックの問題は、ちょっと詰め込みすぎた感じがしますが
一番の狙いは「hh:mm:ss」の間違いに気づくかどうかでした。


この誤りの対策としては、単体テストの前後や、結合試験の完了時ぐらいに、
小文字の「hh」で全文検索や、リテラル検索を掛けておくというのが良いと思います。
FindBugsCheckstyleのルールを自分で書ければ、それに越したことはないんですけどね。


ちなみにDateFormatのインスタンスをDFとかdfとした所に
若干のツッコミがありましたが (^^;
確かに適切な変数名じゃないので、これも意味を踏まえてつけておいてください。


では、そろそろ解答です。

解答

問題箇所は、以下の行です。

int length = str.getBytes().length;

引数なしでString#getBytesを呼び出していますが、
ここには必ず "Windows-31j" とか "UTF-8" などのエンコードを指定するべきです。


エンコードを指定しない場合、実行環境のデフォルトエンコードが指定され、
WindowsマシンならWindows-31jLinuxならeuc-jpなどで実行されてしまうでしょう。


たとえば、strに「あ」が入ってきた場合、
Windows-31jeuc-jpなら、lengthは「2」になりますが、UTF-8なら「3」になります。


半角カタカナの「ア」が入ってきた場合
Windows-31jは「1」、euc-jpは「2」、UTF-8は「3」となります。


さらに、Windows-31jでは扱えないような「㏠」が入ってきた場合
Windows-31jeuc-jpでは「?」に文字化けするので、lengthは「1」になり、UTF-8では「3」になります。


このように、getBytesの引数が指定されず、エンコード不定になった状態では
長さチェックの意味がありません。


だから、getBytesでは、必ずエンコードを指定するべきなのです。

int length;
try {
	length = str.getBytes("Windows-31j").length;
} catch (UnsupportedEncodingException uee) {
	throw new RuntimeException(uee);
}

発生する可能性のないUnsupportedEncodingExceptionは、無視してしまっても良いんですが、
万が一、タイプミスをしてしまっても、単体テスト時点で気づけるように
printStackTraceするか、RuntimeExceptionでラッピングして投げてしまうのが良いでしょう。


また、String#getBytesと対になる、new String(byte[])にも同じことが言えます。
第二引数でエンコードを指定してしなければ、デフォルトエンコードが利用されてしまうため
必ずエンコードを指定するようにしましょう。

まとめ

それでは、今回の問題をおさらいしておきましょう。


ちなみに、
本来なら、事前に文字コードチェックなどを行うべきなのですが
それについて説明すると長くなるので、また次回以降に回すことにしますね。