レビューで鍛えるJavaコーディング力 その２（バリデーション）
前回の問題は、そこそこ好評だったようで、嬉しい限りです。
これからも、週に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」で全文検索や、リテラル検索を掛けておくというのが良いと思います。
FindBugsかCheckstyleのルールを自分で書ければ、それに越したことはないんですけどね。
ちなみにDateFormatのインスタンスをDFとかdfとした所に
若干のツッコミがありましたが (^^;
確かに適切な変数名じゃないので、これも意味を踏まえてつけておいてください。
では、そろそろ解答です。
解答
問題箇所は、以下の行です。
int length = str.getBytes().length;
引数なしでString#getBytesを呼び出していますが、
ここには必ず "Windows-31j" とか "UTF-8" などのエンコードを指定するべきです。
エンコードを指定しない場合、実行環境のデフォルトエンコードが指定され、
WindowsマシンならWindows-31j、Linuxならeuc-jpなどで実行されてしまうでしょう。
たとえば、strに「あ」が入ってきた場合、
Windows-31jとeuc-jpなら、lengthは「2」になりますが、UTF-8なら「3」になります。
半角カタカナの「ｱ」が入ってきた場合
Windows-31jは「1」、euc-jpは「2」、UTF-8は「3」となります。
さらに、Windows-31jでは扱えないような「㏠」が入ってきた場合
Windows-31jとeuc-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[])にも同じことが言えます。
第二引数でエンコードを指定してしなければ、デフォルトエンコードが利用されてしまうため
必ずエンコードを指定するようにしましょう。
