PHP rejestracja PDO

Question

Witam jestem nowym użytkownikiem tego forum.

Wiem że takiego typu posty już były lecz chciałbym aby ktoś mógł ocenić kod i napisał swoje spostrzeżenia oraz jakie poprawki można wprowadzić w tym kodzie.

Z góry dziękuję.

<?php
session_start();
require_once 'database.php';

if(isset($_POST['login']))
{
	function spr_login($login)
	{
		if((strlen($login)<3) || (strlen($login)>20))
		{
			return true;
		}
		else
		{
			return(!ctype_alnum($login)) ? true : false;
		}
	}
	function spr_password($pass, $pass2)
	{
		if((strlen($pass)<8)||(strlen($pass)>20))
		{
			return true;
		}
		else
		{
			return($pass!=$pass2) ? true : false;
		}
	}
	function spr_email($email)
	{
		$emailS = filter_var($email, FILTER_SANITIZE_EMAIL);
		
		return((filter_var($emailS, FILTER_VALIDATE_EMAIL)==false)||($emailS!=$email)) ? true : false;
	}
	
	function check_login($login, $db)
	{		
		$userQuery = $db->prepare('SELECT user FROM uzytkownicy WHERE user=:login');
		$userQuery->bindValue(':login', $login, PDO::PARAM_STR);
		$userQuery->execute();
		
		$user = $userQuery->fetch();
		return($user['user']==$login) ? true : false;
	}
	
	function check_email($email, $db)
	{		
		$userQuery = $db->prepare('SELECT email FROM uzytkownicy WHERE email=:email');
		$userQuery->bindValue(':email', $email, PDO::PARAM_STR);
		$userQuery->execute();
		
		$user = $userQuery->fetch();
		return($user['email']==$email) ? true : false;
	}
	
		$login = $_POST['login'];
		$pass = $_POST['pass'];
		$pass2 = $_POST['pass2'];
		$email = $_POST['email'];
		
		$okey = true;
		
		
		if(check_login($login, $db))
		{
			$okey = false;
			$_SESSION['e_nick'] = "Podany Login jest zajety";
		}
		
		if(check_email($email, $db))
		{
			$okey = false;
			$_SESSION['e_mail'] = "Podany E-mail jest zajety";
		}
		if(spr_login($login))
		{
			$okey = false;
			$_SESSION['e_nick'] = "Podany Login jest nieprawidłowy lub za krótki ";
		}
		if(spr_email($email))
		{
			$okey = false;
			$_SESSION['e_mail'] = "Adres e-mail jest nieprawidłowy";
		}
		if(spr_password($pass, $pass2))
		{
			$okey = false;
			$_SESSION['e_pass'] = "Hasło jest nieprawidłowe lub hasła nie są identyczne";
		}
		
		if($okey == false)
		{
			header('Location: regi.php');
			exit();
		}
		else
		{
			$password = password_hash($pass, PASSWORD_DEFAULT);
			
				$add = $db->prepare('INSERT INTO uzytkownicy(id,user,pass,email) VALUES(NULL, :login, :password, :email)');
                $add->bindValue(":login", $login, PDO::PARAM_STR);
                $add->bindValue(":password", $password, PDO::PARAM_STR);
                $add->bindValue(":email", $email, PDO::PARAM_STR);
                $add->execute();
				
				echo "dzia";
		}
	

}
else
{
	header('Location: regi.php');
	exit();
}





?>

Answer 1

Podstawa, skoro masz takie coś

function spr_login($login)
    {
        if((strlen($login)<3) || (strlen($login)>20))
        {
            return true;
        }
        else
        {
            return(!ctype_alnum($login)) ? true : false;
        }
    }

Gdzie if zwraca wartość, to poco pisać to else {}? 
nie prościej od razu walnąć return? 
skoro if zwraca wartość i nie przejdzie, to jasne że wykona sie to, co masz w else, ja bym to zrobił tak

function spr_login($login)
    {
        if((strlen($login)<3) || (strlen($login)>20))
        {
            return true;
        }
        return(!ctype_alnum($login)) ? true : false;
        
    }

Do tego staraj się przechodzić na programowanie obiektowe, z używaniem klas 
Mógłbyś mieć tutaj osobną klasę do walidacji, której metody byś po prostu wywoływał, aktualnie tracisz na czytelności, bo pchasz całość w jedno miejsce
Staraj się też robić jak najkrótsze ciała if

jeśli możesz zrobić 

if(poprawny warunek)
{
// dużo kodu
}
else 
{
//jedna linia albo dwie
}

lepiej zrobić
 

if(niepoprawny warunek)
{
//jedna linia albo dwie
//return, albo exit, zależy od sytuacji, lepszy return
}
//normalny kod, bo masz pewność że niepoprawny warunek nie przeszedł

To też pozwala zwiększyć czytelność i zaoszczędzić kodu
Weź pod uwage to, że nie zawsze drugi programista będzie sprawdzał if'a na 300 linii, aby sprawdzić czy na końcu jest jakieś else

Dodatkowo poczytaj o PSR - zasady dobrego programowania w PHP
psr 1 i psr 2 tyczą się formatowania, czyli jak nazywać (z jakich liter), jakie klamry w których miejscach 
niby działa bez tego, ale to taki standard stworzony po to, aby każdy pisał kod jak inni, bez zabawy w nauke czyjegoś stylu 

Po ogarnięciu obiektowości, nawet w podstawowym stopniu, polecam przejść na frameworki, np Symfony

Jeśli masz wystarczająco dużo czasu, posprawdzaj inne języki, takie przeskoki sporo dają, np jeśli zajrzysz do języka typowo obiektowego, jak C# albo Java, trochę prościej będzie ci ogarnąć obiektowość, bo tam jest to wymuszone, PHP pozwala pisać kod tak jak chcesz i nie czujesz się zobowiązany pisać obiektowo 

To tak jak z nauką języków, jeśli nie musisz się uczyć języka to uczysz się go dużo wolniej, niż jest on wymagany do zrobienia czegokolwiek 
Przykładowo w Unity3D bez używania typów generycznych za dużo nie zrobisz (szybciej się nauczysz tego elementu niż robiąc jakieś gówno-ćwiczenia z pojedynczych elementów)
A w UK za dużo nie zrobisz, bez umiejętności dogadania się po angielsku (szybciej się nauczysz angielskiego niż siedząc w polsce)

Ogólnie polecam robić duże projekty, na tyle ambitne żeby później narzekać "projekt nie nadaje sie do dalszego rozwoju, bo zrobiłem błąd w podstawie, następnym razem zaprojektuje to lepiej"
Przykładowo w pierwszym większym projekcie przy ok 500 liniach nie dało sie nic dodać, bo co chwile całość sie wysypywała 
W drugim większym ok 1000 to był już taki max, bo dalej już się gubiłem i też miałem problemy z dopisaniem czegoś 
Teraz mam 2600 linii i myślę, że jeszcze 2 razy tyle moge spokojnie dopisać 

Sporo pomagają też wzorce, ale to jak już będziesz czuć się swobodnie w klepaniu kodu

Comments

Dziękuję serdecznie za rady.