2011-05-12 8 views
2

私は、クラスプロパティに値があるかどうかを判断するphpクラスメソッドを持っています。それが何らかの値を保持していれば、$ this-> errorクラスのプロパティを検証して繰り返します。ここで私が使用しているクラスメソッドです。複数のif条件を使用するより良い方法はありますか?

public function validate() { 
    if(!empty($this->name)) { 
     if(!preg_match('/^[a-zA-z ]{3,50}$/',$this->name)) { 
      $this->error['name'] = 'Name should be valid letters and should be between 3 and 25 characters'; 
     } 
    } 
    if(!empty($this->email)) { 
     if(!filter_var($this->email,FILTER_VALIDATE_EMAIL)) { 
      $this->error['invalidEmail'] = 'Invalid email address'; 
     } 
     if(empty($this->userId) && $this->emailCount($this->email)) { 
      $this->error['emailExist'] = 'Email already exist'; 
     } 
    } 
    if(empty($this->userId) && !empty($this->password)) { 
     if(strlen($this->password) < 5 || strlen($this->password > 40)) { 
      $this->error['password'] = 'Password length should be between 5 and 40 characters'; 
     } 
    } 
    if(!empty($this->userId) && !empty($this->newPassword)) { 
     if(strlen($this->newPassword) < 5 || strlen($this->newPassword > 40)) { 
      $this->error['password'] = 'Password length should be between 5 and 40 characters'; 
     } 
    } 
    if(!empty($this->pPhone)) { 
     if(!preg_match('/^[0-9]{5,10}$/',$this->pPhone)) { 
      $this->error['invalidpPhone'] = 'Invalid primary phone number'; 
     } 
    } 
    if(!empty($this->sPhone)) { 
     if(!preg_match('/^[0-9]{5,10}$/',$this->sPhone)) { 
      $this->error['invalidsPhone'] = 'Invalid secondary phone number'; 
     } 
    } 
    return (empty($this->error)) ? true : false;  
} 

私は考えて、ここでの条件は非常に良いではない場合の多くを使用していた、私は上記の条件を決定し、より良い方法でコードを書き換えることができ、他の方法はありますか?

答えて

3

可変変数を使用してコードをループすることができます。しかし、これはあなたのコードのための標準化された検証スキームのいくつかの並べ替えを考え出す必要があります意味

$validateFields = array('email', 'username', 'userId'); //list of fields to validate 
$rules = array(
    'username' => array('type'=> 'regex', 'rule' => '/^[a-zA-z ]{3,50}$/'), 
    'email' => array('type' => 'filter', 'rule' => 'FILTER_VALIDATE_EMAIL') 
); 

foreach ($validateFields as $field) { 
    if (isset($rules[$field])) { 
     switch ($rules[$field]['type']) { 
      case 'regex' : 
       if(!preg_match($rules[$field]['type']['rule'],$this->$field)) { 
        $this->error[$field] = ucfirst($field) . ' should be valid letters and should be between 3 and 25 characters'; 
       } 
       break; 

      case 'filter' : 
       if(!filter_var($this->$field, $rules[$field]['type']['rule'])) { 
        $this->error[$field] = 'Invalid email address'; 
       } 
       break 

      //more cases 
     } 
    } 
} 
return (empty($this->error)) ? true : false; 

は、この例では、フィールドごとに一つのルールを使用しますが、簡単にごとに複数のルールを使用するようにそれを拡張することができるはずですフィールド。

最初はすべてのルールを設定する必要がありますが、クラスに別のプロパティを追加するだけで検証は増加しません。

+0

これは私の意見ではより良い選択肢であり、読むのも簡単です。どうもありがとうございました。 –

+1

@IbrahimAzharArmarはcakePHP検証システムを見ています。それはほぼ同じ原則です。 – JohnP

+0

@JonhP、例のデモンストレーションを行っている特定の記事があります。それは私には有益かもしれません。 –

0

あなたはであなたを助ける場合の条件あなたがブロック

場合であればブロック

ライン ケース1

ブロック

かのケース2 を選択する場合に使用できるかどうかのレ使いたい場合より良い方法

1

私は、この特定の部分コードを実質的により良い方法で書き換えることはできないと思います。

しかし、より大きな画像を見ると、改善が可能です。クラス変数を見ると、これはおそらくフォームクラスか、ある種のモデルインスタンスであり、データを保存する前にそのデータを検証しようとしています。検証ロジックを別のクラスに一般化して抽象化することができます。インスピレーションのために、さまざまなPHPフレームワーク(Symphony、CakePHPなど)がフォームとモデル検証をどのように処理するかを見てみましょう。

+0

私は非常に不一致です。彼はそれをチェックする必要があるそれぞれの事柄のための関数に分けることができます。 「$ this-> error ['password'] = 'パスワードの長さは5〜40文字である必要があります'; "コード内で2回、実質的に改善できないと判断します。パスワードの動作方法を変更した場合、その例でコードを2回コピーして貼り付ける必要があります。 – Anther

+1

パスワードチェックを別の機能に分割するだけでは、大幅な改善とは言えません。 –

0

私はあなたがこの複数のIf文を避けることができるとは思わないが、これはがフィールドを検証する場所であると考えている。 multimpleため

他の代替の文がSwtich声明

だった場合、私はあなたがスイッチとして文句を言わない引数として文字列を取る引数として文字列を渡しているとしてそれは、このような状況のために実現可能であることを確認していません、入力として整数のみが使用されます

+0

スイッチは文字列も引数として受け入れます。 –

0

これは私の人生の仕事ではありませんが、それ以降はコンパイルしていませんが、自分で繰り返したいと思っている道ですあなたのコードで。

ええ、これはあなたの現在の状況に非常に近く、私はIDチェックや他のさまざまなコードを失い、再実装する必要があります。

また、私は、これはすでにクラス内にあることを前提にこれをやっている、とあなたはクラス変数にアクセスしている最初のあなたが定義や、より良いにあなたのエラー文字列を抽出しなければならない時$this->name

<?php 

public function validate() { 
    //You can place your simple ID checks around these wherever they're supposed to go, I lost them ;P. 
    //This code probably won't compile without errors, but it's to give you a much neater idea. 
    //Never ever repeat code !!! Your life becomes so much better :] 
    $this->validate_name($this->name); 
    $this->validate_email($this->email); // I didn't complete this one.. but this is just all for an idea 


    $this->validate_phone_number($this->pPhone,'Primary'); 
    $this->validate_phone_number($this->sPhone,'Secondary'); 

    return (empty($this->error)) ? true : false;  
} 

function validate_password($password){ 
    !empty($password)) { 
     if(strlen($password) < 5 || strlen($password > 40)) { 
      $this->error['password'] = 'Password length should be between 5 and 40 characters'; 
     } 
} 

function validate_name($name){ 
     if(!preg_match('/^[a-zA-z ]{3,50}$/',$name)) { 
      $this->error['name'] = 'Name should be valid letters and should be between 3 and 25 characters'; 
     } 
} 

function validate_phone_number($number,$type){ 
    if(!preg_match('/^[0-9]{5,10}$/',number)) { 
      $this->error['invalidsPhone'] = "Invalid $type phone number"; 
     } 
} 
1

、クラス定数。

define('ERR_BAD_NAME','Name should be valid letters and should be between 3 and 25 characters'); 
define('ERR_BAD_EMAIL','Invalid email address'); 
define('ERR_EMAIL_IN_USE','Email already exist'); 
define('ERR_BAD_PASSWD','Password length should be between 5 and 40 characters'); 
define('ERR_BAD_PRIMARY_PHONE','Invalid primary phone number'); 
define('ERR_BAD_SECONDARY_PHONE','Invalid primary phone number'); 

public function validate() { 
    if(!empty($this->name)) { 
     if(!preg_match('/^[a-zA-z ]{3,50}$/',$this->name)) { 
      $this->error['name'] = ERR_BAD_NAME; 
     } 
    } 
    if(!empty($this->email)) { 
     if(!filter_var($this->email,FILTER_VALIDATE_EMAIL)) { 
      $this->error['invalidEmail'] = ERR_BAD_EMAIL; 
     } 
     if(empty($this->userId) && $this->emailCount($this->email)) { 
      $this->error['emailExist'] = ERR_EMAIL_IN_USE; 
     } 
    } 
    if(empty($this->userId) && !empty($this->password)) { 
     if(strlen($this->password) < 5 || strlen($this->password > 40)) { 
      $this->error['password'] = ERR_BAD_PASSWD; 
     } 
    } 

    if(!empty($this->pPhone)) { 
     if(!preg_match('/^[0-9]{5,10}$/',$this->pPhone)) { 
      $this->error['invalidpPhone'] = ERR_BAD_PRIMARY_PHONE; 
     } 
    } 
    if(!empty($this->sPhone)) { 
     if(!preg_match('/^[0-9]{5,10}$/',$this->sPhone)) { 
      $this->error['invalidsPhone'] = ERR_BAD_PRIMARY_PHONE; 
     } 
    } 
    return (empty($this->error)) ? true : false;  
} 

第2のステップは別々のプライベートメソッドにテストをリファクタリングすることであり、もし、ブロックを取り除く:あなたはバリデータクラスにvalidate_の*メソッドを抽出すべき第三のリファクタリング段階では

private function validate_name($name) { 
    return = empty($name) || preg_match('/^[a-zA-z ]{3,50}$/',$name); 
} 

private function validate_phone($phone) { 
    return empty($phone) || preg_match('/^[0-9]{5,10}$/',$phone); 
} 

private function validate_email($email) { 
    return empty($email)) || filter_var($email,FILTER_VALIDATE_EMAIL); 
} 

private function emailInUse($email, $userId) { 
    if(!empty($email)) { 
     if(empty($this->userId) && $this->emailCount($this->email)) { 
      $this->error['emailExist'] = ERR_EMAIL_IN_USE; 
     } 
    } 
} 

private function validate_userPassword($userId, $password) { 
    $passwordLen=strlen($this->password); 
    return (empty($this->userId) && !empty($this->password)) || 
      (5 <= $passwordLen) && (40 >= $passwordLen); 
} 

private function lor_error($type, $message) { 
    $this->error[$type] = $message; 
    return false; 
} 

public function validate() { 
    $isValid = true; 
    $isValid &= $this->validate_name($this->name) || 
       $this->logError('name',ERR_BAD_NAME); 
    $isValid &= $this->validate_email($this->email) || 
       $this->logError('invalidEmail',ERR_BAD_EMAIL); 
    $isValid &= $this->emailInUse($this->userId, $this->email) || 
       $this->logError('emailExist',ERR_EMAIL_IN_USE); 
    $isValid &= $this->validateUserPassword($this->userId, $this->password) || 
       $this->log('password', ERR_BAD_PASSWD); 
    $isValid &= $this->validate_phone($this->pPhone) || 
       $this->log('invalidpPhone',ERR_BAD_PRIMARY_PHONE); 
    $isValid &= $this->validate_phone($this->sPhone) || 
       $this->log('invalidsPhone',ERR_BAD_SECONDARY_PHONE); 
    return $isValid; 
} 

メインクラスから検証タスクを切り離す。この後者の段階は明らかに(適切なログクラスを定義する負担として)あなた次第です。

関連する問題