2009-03-19 10 views
1

私は以下の小さなコードを持っており、ベストプラクティス/コードの保守性などに関してあなたがどのような種類のものを変更するのか不思議でした。ベストプラクティス/メンテナンスのために私のコードで何を変更しますか?

function _setAccountStatus($Username, $AccountStatus) 
{ 
if ($Username == '' || ($AccountStatus != 'Active' || $AccountStatus != 'Banned' || $AccountStatus != 'Suspended')) { 
    // TODO: throw error here. 
} 

$c1 = new Criteria(); 

$c1->add(UsersPeer::USERNAME,$Username); 

$rs = UsersPeer::doSelect($c1); 

if (count($rs) > 0) { 
    $UserRow = array_pop($rs); 

    $UserRow->setAccountStatus($AccountStatus); 

    try { 
     $UserRow->save(); 
    } catch (PropelException $e) { 
     return false; 
    } 

    return true; 
} 

return false; 
} 

答えて

2

私はあなたのif文で代わりに$ユーザー名のempty() == '' を使用します。以前はpropelを使用していませんでしたが、このメソッドをUserオブジェクト自体に置いて、別個のオブジェクトによって実行されるユーザーオブジェクトの取得と保存をすることをお勧めします。擬似コードはこのようなものです。

$ user = userManager-> getUser($ username); $ user-> setAccountStatus($ accountStatus); $ userManager-> saveUser($ user);

0

コードをより読みやすくするために、最後にfalseを返す前のelse節が優先されます。