Open sica07 opened 5 years ago
In PHP exista conventia ca numai constantele sa fie scrise cu majuscule. https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/AuthenticateClass.php#L12 https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/AuthenticateClass.php#L42-L45 https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/AuthenticateClass.php#L30 Folositi return type checking (design-by-contract): daca primesc un anume input voi returna un anume output. Functia de mai sus returneaza doar true sau false deci ati putea scrie: function authenticateUser(): bool (cand nu avem return sau se pot returna mai multe tipuri de date, folositi : void; cand retrun value e null SAU un anume tip de data folositi : ?array, : ?App\Models\User etc. Folositi else cat mai rar cu putinta. https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/AuthenticateClass.php#L50-L58 De exemplu , mai sus, daca intra pe if nu va mai intra pe else, deci else nu mai e necesar.
function authenticateUser(): bool
: void
: ?array
: ?App\Models\User
else
if
Verificarea valorilor boolean se poate face fara ===: https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/RegisterClass.php#L37 Cel mai simplu ar fi o verificare de genul if(!$this->checkEmailExists() ).
===
if(!$this->checkEmailExists() )
https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/RegisterClass.php#L28-L33 could be refactor to:
return $stmt->fetch();
https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/RegisterClass.php#L35-L46 could be refactor to:
function registerUserInDb(): ?integer { if ($this->checkEmailExists()) { return; //return null } $sql = "INSERT INTO `users` (email,password,username) VALUES(?,?,?)"; $hashedPassword = password_hash($this->password, PASSWORD_DEFAULT); $stmt = $this->pdo->prepare($sql); $stmt->execute([$this->email, $hashedPassword, $this->username]); return $this->pdo->lastInsertId; //99% you will needed later //OR $stmt = $this->pdo->query("SELECT LAST_INSERT_ID()"); return $stmt->fetchColumn();
Atat deocamdata. Oricum, ati facut o treaba foarte faina! Bravo :+1:
In PHP exista conventia ca numai constantele sa fie scrise cu majuscule. https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/AuthenticateClass.php#L12 https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/AuthenticateClass.php#L42-L45 https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/AuthenticateClass.php#L30 Folositi return type checking (design-by-contract): daca primesc un anume input voi returna un anume output. Functia de mai sus returneaza doar true sau false deci ati putea scrie:
function authenticateUser(): bool
(cand nu avem return sau se pot returna mai multe tipuri de date, folositi: void
; cand retrun value e null SAU un anume tip de data folositi: ?array
,: ?App\Models\User
etc. Folositielse
cat mai rar cu putinta. https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/AuthenticateClass.php#L50-L58 De exemplu , mai sus, daca intra peif
nu va mai intra peelse
, decielse
nu mai e necesar.Verificarea valorilor boolean se poate face fara
===
: https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/RegisterClass.php#L37 Cel mai simplu ar fi o verificare de genulif(!$this->checkEmailExists() )
.Refactoring suggestions:
https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/RegisterClass.php#L28-L33 could be refactor to:
https://github.com/RaduTodor/TestingPHP/blob/0a5162811182348031e94ea9a25087a685fa22f6/app/classes/RegisterClass.php#L35-L46 could be refactor to:
Atat deocamdata. Oricum, ati facut o treaba foarte faina! Bravo :+1: