Хорошая ли реализация проверки ввода полей?

Question

[Константин Гулла]

Я только учусь и для проверки ввода данных в полях логин, пароль и почта написал вот такой код

if (empty($login)) { $msg_err = "1"; } else { $msg_err = ""; }
if (empty($password)) { $msg_err = $msg_err."2"; } else { $msg_err = $msg_err.""; }	
if (empty($email)) { $msg_err = $msg_err."3"; } else { $msg_err = $msg_err.""; }		
header( 'Refresh: 0; url=/reg.php?msg='.$msg_err);

Всё прекрасно работает, и по коду передаваемого $msg_err вывожу ошибку о том, какие строки были не введены.
У пользователей тостера хотел спросить, как вам этот код в плане выполнения поставленной задачи? Т.к. меня терзают сомнения, что можно сделать и лучше. Жду критики и советов.

Comments

[Александр Шурыгин]

Дырявая форма. Вас взломают

Answer 1

[Сергей Протько]

Это очень плохо, привязываться к номерам строк и т.д. У вас есть названия полей, по которым можно потом мэпить ошибки на поля при выводе формы.

В целом же код свою работу выполняет. Единственное что ваша реализация ограничена одним сообением об ошибке. То есть если у вас не заполнено первое и третье поле, будет выведено сообщение только о том что не заполнено третье поле, и пользователю придется еще раз отправлять форму что бы узнать о том, что у него и первое не заполнено.

Советы аля "воспользуйтесь какой нормальной библиотекой для валидации данных, приводите данные к виду объектов" и т.д. думаю вам пока не сильно полезны будут.

Comments

[AlexP11223]

Там же конкатенация строк во всех, кроме первого: $msg_err . "3"
так что по идее должно все вывести.

Я кстати не понял зачем там else вообще.

[Сергей Протько]

@AlexP11223, меня присвоение смутило... не заметил там конкатенации, подумал что она и не делается.

Тогда реализация вообще бесполезна ибо не очивидно как детектить где именно ошибка произошла.

[Сергей Протько]

@AlexP11223, а else там судя по всему для инициализации переменной $msg_err...

[Константин Гулла]

@AlexP11223 вы меня правильно поняли, всё выводится как и задумывали. Я вот про else думал, но решил перестраховаться.

[Сергей Протько]

@Koteika_Tut, достаточно инициализировать переменную перед этими ивами. И заменить $msg_err = '' . $msg_err на $msg_err .= ''.

Но вообще я бы рекомендовал вам работать напрямую с массивом, а не биндить на переменные (тут могут тоже быть ошибки), и формировать результат тоже как массив - имя, ошибка. Если массив пуст - значит ошибок нету и все валидно.

[Константин Гулла]

@Fesor благодарю за намёк на массивы, думаю перепишу под них вывод ошибок.

Answer 2

[Александр Гончаров]

Можно было бы исполнить и так:

if (!empty($login)) {
	$msg_err = null; 
	if (!empty($password)) {
		$msg_err = null; 
			if (empty($email)) {$msg_err = "3";} 
			} else {$msg_err = "2"; } 
	} else {$msg_err = "1"; } 
header( 'Refresh: 0; url=/reg.php?msg='.$msg_err);
}

Comments

[Александр Шурыгин]

Дырявая форма. Вас взломают

Answer 3

[zenaku]

Сделал бы так:

if  (isset($login,$password,$email)  {
   heder('location: /reg.php'); exit;
} else {
   echo 'Вы не заполнили все поля'; // иди же heder('location: /reg.php?msg_err=1'); exit;
}

Или

if ($login && $password && $email) {
   heder('location: /reg.php'); exit;
}
...

Comments

[zenaku]

Только первый будет не совсем корректен, так как переменные уже прошли проверку при присваивании от $_POST, поэтому они существуют.
А показывать пользователю, какое поле он не заполнил на php не стоит. Это на JS с html5 надо делать. А уж если он проходит ту защиту от 'дурака', то задайтесь вопрос, зачем это ему...

[Александр Шурыгин]

Дырявая форма. Вас взломают

[zenaku]

@duderu, это был пример, а не код для использования с проверкой переменных и отлавливание ошибок

[Александр Шурыгин]

@zenaku понятно

Answer 4

[Александр Кубинцев]

Я рекомендую пользоваться "формами", как например здесь https://github.com/kryoz/sociochat/blob/master/src... , хотя реализация достаточно примитивная

try {
   $form = (new Form())
      ->import($request)
      ->addRule(PropertiesDAO::NAME, Rules::namePattern(), $lang->getPhrase('InvalidNameFormat'))
      ->addRule(PropertiesDAO::TIM, Rules::timPattern(), $lang->getPhrase('InvalidTIMFormat'))
      ->addRule(PropertiesDAO::SEX, Rules::sexPattern(), $lang->getPhrase('InvalidSexFormat'));
} catch (WrongRuleNameException $e) {
   $this->errorResponse($user, ['property' => $lang->getPhrase('InvalidProperty')]);
   return;
}

Такая форма валидации гораздо легче читается и управляется.
Сами правила ничто иное, как анонимные функции, но можно и типовые случае в фабрику запихнуть
https://github.com/kryoz/sociochat/blob/master/src...