Что бы вы изменили в моем коде для передового опыта / обслуживания?

Question

Ниже приведен небольшой фрагмент кода, и мне было любопытно, какие типы вещей вы бы изменили в связи с рекомендациями / поддержкой кода и т. Д.

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;
}

Answer 1

Я бы использовал empty () вместо $ Username == '' в вашем операторе if. Раньше я не использовал propel, но я бы предпочел, чтобы этот метод использовался для самого объекта User, а выборка и сохранение объекта user выполнялись отдельным объектом. Псевдокод будет примерно таким.

$ user = userManager-> getUser ($ username); $ User-> setAccountStatus ($ accountStatus); $ UserManager-> saveUser ($ пользователю);

Answer 2

Предложение else перед последним возвращением false было бы предпочтительным, просто чтобы сделать код более читабельным.