重複するチェックをするべきかどうか

Question

お世話になります。

下記のような状況のときに、
重複するチェックをするべきかどうか
で悩んでおります。

例えば、パスワードの登録時の簡単なチェッカーを作る際、
１．古いパスワードが入力されているかどうか
２．新しいパスワードが入力されているかどうか
３．新しいパスワードの再入力されているかどうか
４．古いパスワードとDBに保持しているパスワードが一致するかどうか
５．新しいパスワードと再入力されたパスワードが一致するかどうか
のような手順になるかと思います。

この４、５の一致チェックの時に、再度nullチェックをかけるかどうか、です。

また、なぜこういった考えになったかにつきましては
再度nullチェックをかけた方がリスク回避にもなり、必要そうだ、とは思うのですが
どうしても冗長な処理になってしまうので、一般的にはどういった処理にするのが
好ましいのかわからず質問させて頂きました。

お手数おかけ致しますが、よろしくお願い申し上げます。

以下コードです。

public class PasswordChecker{
  public String getInputParamErrorMsg(PasswordParam inputParam, String db){
    final StringBuilder result = new StringBuilder();
    // 未入力チェック
    result.append(getInvalidInputParam(inputParam));
    // 一致チェック
    result.append(getNonconformityParam(inputParam, db));
    return result.toString();
  }
  
  private String getInvalidInputParam(PasswordParam inputParam){
    final StringBuilder result = new StringBuilder();
    if(isInvalid(inputParam.oldPassword)){
      result.append("古いパスワードを入力して下さい。");
    }
    if(isInvalid(inputParam.newPassword)){
      result.append("新しいパスワードを入力して下さい。");
    }
    if(isInvalid(inputParam.rePassword)){
      result.append("新しいパスワードを再入力して下さい。");
    }
    return result.toString();
  }
  
  private String getNonconformityParam(PasswordParam inputParam, String db){
    final StringBuilder result = new StringBuilder();
    if(isMatch(inputParam.oldPassword, db)){
    result.append("古いパスワードが一致しません\n");
    }
    if(isMatch(inputParam.newPassword, inputParam.rePassword)){
    result.append("入力された新しいパスワードが一致しません。\n");
    }
    return result.toString();
  }
  
  private boolean isInvalid(String param){
    if(param != null && param.length() != 0){
      return false;
    }
    return true;
  }
  
  private boolean isMatch(String basic, String target){
    // 下記個人的ぼやきです。
    // このメソッドだけでみると下記チェックは必須だが、流れからすると不要になるように思える。
    // 多人数プロジェクトの場合は、必須になるような気もするので、やはり必要？
    // 他の人から、ここでもnullチェックしている理由がわからない、となったらそれはそれで混乱を招くような気もする。
    if(isInvalid(basic)){
      return false;
    }
    if(basic.equals(target)){
      return true;
    }
    return false;
  }
}

public class PasswordParam {
  public String oldPassword;
  public String newPassword;
  public String rePassword;
}

Answer 1

結論から言えば、今回の実装に関しては必要だと思います。
nullチェックを重複して行うかどうかは、「実行する段階でnullでないことが保証されているかどうか？」がポイントです。

• nullでない場合のみにisMatch()が実行される

→isMatch()実行時にはnullでないことが保証されているので不要

• nullの場合にもisMatch()が実行される

→isMtach()実行時にnullである可能性があるので、必要

質問のコードの場合、未入力チェックと一致チェックが独立しており、未入力チェックに引っかかった場合でも一致チェックが行われます。
isMatch()実行時にnullである可能性があるため、チェックする必要があると思います。

Comments

[Batoh33789]

ご助言有難うございます。
仰る通り、提示したコードでは、isMatchメソッドでnullチェックをしないと例外が発生します。
提示したコードがあまり良くなく申し訳ありませんでした。

Answer 2

自分的には、関数を呼ぶ時のルールの決めの問題だと思います。
チェック処理が冗長な処理になるのはしょうがないような気がします。

呼び出し側で未入力等のＤＢアクセスの必要が無い単項目チェックがされている前提の場合は、
NULLチェックは必要ないです。

呼び出し側でのチェックの保障がない場合は、NULLのチェックをしないといけないと思います。

ライブラリとして他のシステムに提供する場合等は、
常にチェックするかあえて例外が発生するような実装にしますね。

個人的には、質問の例だと入力が空の単項目チェックは予備元ですべきだと思いますが。。。
後チェックのメソッドも一つで良い気がします。複数に別けるメリットが無い気がします。

Comments

[Batoh33789]

ご助言有難うございます。
仰る通り、ルール決めをどうするか、という問題ですね。
また、ご指摘の通り、チェッカーメソッドを二つ用意する必要がないコードになってしまってますね・・。
提示したコードの例があまり良くなく、申し訳ありませんでした。

Answer 3

どういった場合、４，５のタイミングでデータがnullになっているのでしょうか。
nullになる根拠があれば必要ですし、根拠がなければ冗長な処理だと思います。
また、nullチェックを入れることにより、予期せぬ動作をするといったリスクもありますし、
テスト工数も増えてしまいます。

Comments

[Batoh33789]

提示したコードでは、nullになりません。
ただテスト工数が増えるというのは盲点でした。
ご助言有難うございます。