[Coding]複数の条件判定には統一性を持たせる

いったんOKと言ったあとで「やっぱダメ」と言う人、嫌われるよね?
コードでも同じ。

ある判定を行う固まりで、条件に統一性が無いとすごくわかりづらいです。

bool isValidXxxx(string targetValue, bool allowNull, bool allowZero, bool allowWhiteSpace){
   
  //nullチェック
  if(!allowNull && targetValue == null){
    return false;
  }
 
  //ゼロチェック
  if(!allowZero)
  {
    int parsed;
    if( Int32.TryParse(targetValue, out parsed) && parsed==0){
      return false;
    }
  }
 
  //空白のみ・空文字チェック
  if(!allowWhiteSpace && targetValue.Trim.Equals("")){
    return false;
  }
}

void main(){
  var targetValue = "";

  bool result = isValidXxxx(targetValue, true, false, false);  //nullはOKで判定している
  if(targetValue == null){
    result = false;   //(・д・)なぜに
  }
}

26行目のisValidXxxxではnullはOKで判定依頼しているのに、そのあと(27行目)で自分でnullの場合はNGのチェックを行っています。
「こんなコード書かねぇよ」と思うかもしれませんが、nullの場合は特別なメッセージをセットしたいという要件がある場合、書いてしまうことがあるようです。そのような場合でも、条件は一致させておくのが誠意のあるコードです。

bool isValidXxxx(string targetValue, out string message, bool allowNull, bool allowZero, bool allowWhiteSpace){
   
  //nullチェック
  if(!allowNull && targetValue == null){
    message = "null error";
    return false;
  }
 
  //ゼロチェック
  if(!allowZero)
  {
    int parsed;
    if( Int32.TryParse(targetValue, out parsed) && parsed==0){
      return false;
      message = "zero error";
    }
  }
 
  //空文字チェック
  if(!allowWhiteSpace && targetValue.Trim.Equals("")){
    meaage = "white space only";
    return false;
  }
}

void main(){
  var targetValue = "";
  var message = "";

  bool result = isValidXxxx(targetValue, out message, false, false, false);  //nullはNGで判定
  if(targetValue == null){  //nullの場合はメッセージ書き換え
    message="NULLだよ!";
  }
}

そもそもisValidXxxxの作りがイケてないというのは、言ってはいけない。

コメントを残す