Problemy z walidacją OOP Opcje

[Panicz74]

Witam,

Mam problem z formularzem rejestracji. Walidacja działa jedynie w przypadku pola 'username', w pozostałych przypadkach niestety nie. Zupełnie nie mam pojęcia gdzie może być błąd. Może wy coś doradzicie?

register.php

[PHP] pobierz, plaintext 
<?php
 
require_once 'core/init.php';
 
if(Input::exists())
{
	$validate = new Validate();
	$validation = $validate->check($_POST, array(
		'username' => array(
			'required' => true,
			'min' => 2,
			'max' => 20,
			'unique' => 'users'
		),
		'password' => array(
			'required' => true,
			'min' => 6,
		),
		'password_again' => array(
			'required' => true,
			'matches' => 'password'
		),
		'name' => array(
			'required' => true,
			'min' => 2,
			'max' => 20
		),
	));
 
	if($validation->passed())
	{
		echo "Registered";
	}
	else
	{
		print_r($validation->errors());
	}
}
 
?>
[PHP] pobierz, plaintext 

validate.php

[PHP] pobierz, plaintext 
<?php
class Validate
{
	private $_passed = false;
	private $_errors = array();
	private $_db = null;
 
	public function __construct()
	{
		$this->_db = DB::getInstance();
	}
 
	public function check($source, $items = array())
	{
		foreach($items as $item => $rules)
		{
			foreach($rules as $rule => $rule_value)
			{
				$value = $source[$item];
 
				if($rule === 'required' && empty($value))
				{
					$this->addError("{$item} is required");
				}
				else
				{
					$this->_passed = true;
				}
				return $this;
			}
		}
		if(empty($this->_errors))
		{
			$this->_passed = true;
		}
		//return $this;
	}
 
	private function addError($error)
	{
		$this->_errors[] = $error;
	}
 
	public function errors()
	{
		return $this->_errors;
	}
 
	public function passed()
	{
		return $this->_passed;
	}
}
?>
[PHP] pobierz, plaintext 

[Tomplus]

Na moje oko działa ci to "dobrze", bo jak działa w przypadku username, to jeżeli będziesz miał usera złego, a hasło dobre, to będzie ci działać dobrze dla hasła.
Czyli dla każdego poprawnego pola będziesz miał $this->_passed = true; i if($validation->passed()) także będzie spełnione.

[session]

[PHP] pobierz, plaintext 
			foreach($rules as $rule => $rule_value)
			{
				$value = $source[$item];
 
				if($rule === 'required' && empty($value))
				{
					$this->addError("{$item} is required");
				}
				else
				{
					$this->_passed = true;
				}
				return $this;
			}
[PHP] pobierz, plaintext 

Na końcu tej pętli masz return co skutkuje tym, że wykona się tylko 1 raz (czyli dla username w tym przypadku) i zakończy działanie niezależnie od wyniku walidacji. Co więcej użycie w else{ $this->_passed = true; } powoduje, że wystarczy, że jeśli tylko jedno będzie poprawne, a pozostałe złe to i tak wynik będzie true, $this odwołuje się do całego obiektu, a nie pojedynczych wartości. No i na dodatek if($rule === 'required' && empty($value)) sprawdzasz tylko czy w tablicy jest element "required", a nie sprawdzasz, czy ma wartość true czy false.

Wywal to else, a return daj tam gdzie masz zakomentowany.

Ten post edytował session 11.02.2016, 21:53:44

[Riggs]

Dokładnie tak jak pisze @session, przez ten return waliduje się tylko pierwsze pole.

[JakubBab]

Pytanie do Twojego kodu :

Linijka:

[PHP] pobierz, plaintext 
						foreach($rules as $rule => $rule_value)
			{
				$value = $source[$item];
 
				if($rule === 'required' && empty($value))
				{
					$this->addError("{$item} is required");
				}
				..............bla bla bla
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
foreach($rules as $rule => $rule_value)
			{
				$value = $source[$item];
[PHP] pobierz, plaintext 

Powieli tablice zgodnie z liczba iteracji (3 lub 4 w tym przypadku). Czy tak mialo byc i czy to jest dobra praktyka? Nie zabiera to (zbednie) pamieci przez iterowanie 3 or 4 razy tego samego?

Ten post edytował JakubBab 20.02.2016, 14:48:43

[com]

@up co masz na myśli?

[JakubBab]

tablica

[PHP] pobierz, plaintext 
 
foreach($rules as $rule => $rule_value)
			{
				$value = $source[$item];
[PHP] pobierz, plaintext 

tablica $value w tej petli to:

[PHP] pobierz, plaintext 
Array
(
    [required] => 1
    [min] => 2
    [max] => 20
    [unique] => users
)
Array
(
    [required] => 1
    [min] => 2
    [max] => 20
    [unique] => users
)
Array
(
    [required] => 1
    [min] => 2
    [max] => 20
    [unique] => users
)
Array
(
    [required] => 1
    [min] => 2
    [max] => 20
    [unique] => users
)
Array
(
    [required] => 1
    [min] => 6
)
Array
(
    [required] => 1
    [min] => 6
)
Array
(
    [required] => 1
    [matches] => password
)
Array
(
    [required] => 1
    [matches] => password
)
Array
(
    [required] => 1
    [min] => 2
    [max] => 20
)
Array
(
    [required] => 1
    [min] => 2
    [max] => 20
)
Array
(
    [required] => 1
    [min] => 2
    [max] => 20
)
[PHP] pobierz, plaintext 

Jest powielona.

[com]

ok, no to jest źle napisane to fakt

[JakubBab]

[com]

bledna przeróbka tego https://www.youtube.com/playlist?list=PLfdt...CE9mxspdpX_JleP