Ocena kodu napisanego obiektowo

Question

Witam

To znowu ja :D, po ostatnim moim poście z ocena skryptu logowania postanowiłem poszukać jeszcze w internecie informacji jak dobrze pisać obiektowo, jak na razie nie pisałem jeszcze ze wzorcem projektowym postanowiłem napisać w czystym OOP, poczytałem też o zasadzie SOLID, myślę że chociaż trochę z tego skorzystałem, ale mam 1 wątpliwość co do zasady w której kod powinien być "otwarta na rozbudowe i zamknięta na modyfikacje" czy do tego zalicza się dodanie odwołania do wykonana funkcji w już istniejącej i np. chciałbym dodać do metody verification skrypt który zablokuje konto jeżeli ktoś wpisze 4 razy błędne hasło, i dodam samo odwołanie $this->funkcja(); to złamie wtedy zasadę "open/close principle"?

DBmanager.php

<?php

class DBmanager
{
	private $db_host = "localhost";
	private $db_user = "root";
	private $db_password = "";
	private $db_name;
	private $db_connect;
	
	public function __construct($db_name = "test")
	{
		$this->db_connect = new mysqli($this->db_host, $this->db_user, $this->db_password, "$db_name");
	}
	
	public function checkConnection()
	{
		if($this->db_connect->connect_errno)
		{
			throw new Exception($this->db_connect->connect_errno);
		}
		else
		{
			return $this->db_connect;
		}
	}
		
	public function closeConnection($connect)
	{
		$connect->close();
	}
}

?>

AuthC.php

<?php

class AuthC
{		
	private $login;
	private $password;
	private $password2;
			
	public function login($login, $password)
	{		
		$this->login = htmlentities($login);
		$this->password = htmlentities($password);
		$vlogin = new VerificationLogin();
		
		try
		{
			return $vlogin->verification($this->login, $this->password);		
		}
		catch(Exception $e)
		{
			$_SESSION['exceptionLogin'] = $e->getMessage();
			return DATABASE_ERROR;
		}
	}
}

?>

VerificationLogin.php

<?php

class VerificationLogin
{
	private $login;
	private $password;
	private $DBmanager;
		
	public function __construct()
	{
		$this->DBmanager = new DBmanager();
	}
		
	public function verification($login, $password)
	{
		$connect = $this->DBmanager->checkConnection();
		
		$this->login = $connect->real_escape_string($login);
		$this->password = $password;
		
		$query = $connect->query("SELECT id, login, password, last_login FROM users WHERE login='$this->login'");
		
		if($query->num_rows >= 1)
		{
			$row = $query->fetch_assoc();
		
			if(password_verify($this->password, $row['password']))
			{
				$_SESSION['logged'] = true;
				$_SESSION['idUser'] = $row['id'];
				$_SESSION['login'] = $row['login'];
				$_SESSION['lastLogin'] = $row['last_login'];
				return LOGIN_SUCCESS;
			}
			else
			{
				$_SESSION['rememberLogin'] = $this->login; 
				$_SESSION['viewPasswordError'] = true;
				return false;
			}
		}
		else
		{
			return LOGIN_FAILED;
		}
		
		$this->DBmanager->closeConnection($connect);	
	}
}

?>

index.php

session_start();
require_once('class/ClassLoader.php');
require_once('constants.php');
$AuthC = new AuthC();

if(isset($_POST['login']))
{
	if((empty($_POST['login'])) || (empty($_POST['password'])))
	{
		$_SESSION['viewLoginError'] = true;
		$_SESSION['viewPasswordError'] = true;
	}
	else
	{
		switch($AuthC->login($_POST['login'], $_POST['password']))
		{
			case LOGIN_SUCCESS:
				header('Location: templates/informations.php');
				break;
			case LOGIN_FAILED:
				$_SESSION['viewLoginError'] = true;
				break;
			case DATABASE_ERROR:
				$_SESSION['viewExceptionLogin'] = true;
				break;
		}
	}
}

 

Comments

Blokowanie konta powinno odbywać się w tej klasie, w której jest zmienna licząca ilość prób logowania. Problem w tym, że u ciebie wszystkie istotne informację są  w SESSION i nie da się tego całkowicie wyeliminować ( taka ułomność PHP). W dobrym programie obiektowym takie niskopoziomowe struktury powinny być opakowane w klasy bardziej bliższe dziedzinie. U ciebie dane sesji związane z logowaniem są opakowane klasą VerificationLogin. Ponieważ licznik prób logowania również jest związany z logowaniem to również powinien się znaleźć w tej klasie i tam powinno właśnie odbywać się blokowanie konta. Pamiętaj tylko, że gdy w jednej klasie używasz np. "$_SESSION['logged']" to nie możesz użyć tego wywołania w żadnej innej klasie. Jedna klasa powinna opakowywać dane związane z logowaniem, a gdy inne klasy potrzebują informacji o logowaniu, powinny użyć klasy opakowującej. Pamiętaj również, że zasady SOLID służą do redukowania złożoności programu. Jeśli jakaś zasada powoduje większą złożoność to nie należy jej w tym wypadku przestrzegać. Nie powinieneś myśleć w kategoriach "czy ten kod przestrzega SOLID", ale "czy ten kod da się jakoś uprościć".

---

myślałem żeby ten skrypt zrobić w ten sposób: stworzyć nową metodę w klasie VerificationLogin i wywołać ją jeśli ktoś wpisze błędne hasło i przy każdej próbie dodawać +1 do liczby która zostaje zapisana w bazie a gdy zaloguje się to wyzerować ten licznik, a w miarę dobrze ten skrypt wygląda?

Answer 1

PHP to nie do końca moja bajka, ale z tego co widzę to mógłbyś pokusić się o użycie interfejsów oraz wzorców projektowych np. Dependency Injection w tym miejscu:

public function __construct()
    {
        $this->DBmanager = new DBmanager();
    }

Szczegóły pozostawiam do zbadanie Tobie.

Comments

na początku chciałem użyć bez zobaczyć jak to będzie działało.

---

Poprosiłeś o ocenę, więc napisałem tyle ile wiem :) Druga sprawa, że dopiero teraz doczytałem, że chciałeś to na początku zrobić bez wzorców. W takim razie proponuję użycie PDO przy połączeniu, jako początkowe ulepszenie twojego skryptu.

---

jestem w trakcie uczenia się PDO, w między czasie tworzyłem skrypt z wykorzystaniem MySQLI

Answer 2

ref