@dev400

Проверка данных. Насколько целесообразно делать так?

public function validChangeData($array,$uid) {
        if(empty($array['email']))  {
            return false;
        }
        if(!preg_match("/^[a-z0-9](([_\.-]?[a-z0-9]+)*){0,19}@([a-z0-9]([_-]?[a-z0-9]+)?\.){1,3}[a-z]{2,6}$/i", $array['email'])) {
            return false;
        }
        $this->updateUserData($array,$uid);
    }


Два условия, и если не false то вызывается приватный метод, который записывает в базу. Или бред?
  • Вопрос задан
  • 294 просмотра
Решения вопроса 2
mytmid
@mytmid
нормальные люди в тостере хлеб поджаривают :D
Первое условие бесполезно.
Условие лучше поменять местами:
public function validChangeData($array,$uid) {
    $reg = '/[a-z0-9](([_\.-]?[a-z0-9]+)*){0,19}@([a-z0-9]([_-]?[a-z0-9]+)?\.){1,3}[a-z]{2,6}/i';
    if( isset($array['email']) && preg_match($reg, $array['email']) ) {
        $this->updateUserData($array,$uid);
    }
    return false;
    }
Ответ написан
index0h
@index0h
PHP, Golang. https://github.com/index0h
false в вашем случае - это что-то пошло не так. Это не информативно. Используйте исключения.

Что касается проверок - их не достаточно.
Обязательно проверяйте типы аргументов для простых типов, для обьектов - лучше тайп хинтинг. Например что произойдет, если вместо массива вам влетит обьект?

Могу порекомендовать либу для быстрой валидации аргументов: https://github.com/ko-ko-ko/php-assert
В вашем случае проверка будет такая:
/**
 * @param array $array
 * @param int   $uid
 * @throws InvalidArgumentException
 * @throws InvalidArrayException
 * @throws InvalidIntException
 * @throws InvalidIntOrFloatException
 * @throws InvalidNotEmptyException
 * @throws InvalidNotObjectException
 * @throws InvalidRegexpPatternException
 * @throws InvalidStringException
 * @throws NumberNotPositiveException
 * @throws StringNotMatchRegExpException
 */
public function validChangeData($array, $uid)
{
    Assert::assert($array, 'array')->isArray()->notEmpty();
    Assert::assert($uid, 'uid')->int()->positive();
    
    if (!array_key_exists('email', $array)) {
        throw new \InvalidArgumentException('Argument "$array" has no element "email"');
    }
    
    $email = $array['email'];

    Assert::assert($email, 'email')->string()->notEmpty()->match(
        '/^[a-z0-9](([_\.-]?[a-z0-9]+)*){0,19}@([a-z0-9]([_-]?[a-z0-9]+)?\.){1,3}[a-z]{2,6}$/i'
    );

    $this->updateUserData($array, $uid);
}
Ответ написан
Пригласить эксперта
Ответы на вопрос 2
ThunderCat
@ThunderCat Куратор тега PHP
{PHP, MySql, HTML, JS, CSS} developer
if(!empty($array['email'])&&
preg_match("/^[a-z0-9](([_\.-]?[a-z0-9]+)*){0,19}@([a-z0-9]([_-]?[a-z0-9]+)?\.){1,3}[a-z]{2,6}$/i", $array['email']))  {
        $this->updateUserData($array,$uid);       
}

Зачем такой огород городить? Код должен читаться ну, как фраза что-ли. Внятно читаться. "Если емайл не пустой или совпадает с шаблоном - записать изменения".
Ответ написан
hbuser
@hbuser
Что именно вы имеете в виду под бредом? Производительность, красота, логичность, минимальное количество строк кода, отсутствие повторений, ... .
Можно было бы записать немного по-другому, но и то что есть - не сказать, что это бред. Просто код. Обыкновенная проверка.
Это не какой-то требовательный к ресурсам код, не что-то особенное. Повторюсь... просто код.
P.S. Если недостатки на ровном месте искать, то это только если фен-шуй за уши подтягивать...
Ответ написан
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы