Forum PHP.pl

Czy moglibyście zapoznać się z moim kodem rejestracji i logowania użytkowników.
Po ostatniej wpadce ze stronką chciałbym zasięgnąć opinii bardziej doświadczonych programistów, a że nie mam kogo się poradzić pisze dlatego na tym forum.
Oto kod:

[PHP] pobierz, plaintext 
<?php
session_start();
class Uzytkownik
{
    private $bazaDanych;
    //łączenie z bazą danych
    public function polaczZBazaDanych()
    {
        $this->bazaDanych = new mysqli('localhost','uzytkownik','haslo','bazaDanych');
    }
    //reozlaczenie z baza danych
    public function rozlaczZBazaDanych()
    {
        $this->bazaDanych->close();
    }
    //walidacja danych wprowadzonych przez użytkownika
    private function walidacja($dana)
    {
		//do uzupełnienia
        return $dana;
    }
    //rejestracja użytkownika
    public function zarejestrujUzytkownik($login,$email,$haslo1,$haslo2)
    {
		//łączenie z bazą danych
        $this->polaczZBazaDanych();
        //walidacja czterech zmiennych
        $login = $this->walidacja($login);
        $email = $this->walidacja($email);
        $haslo1 = $this->walidacja($haslo1);
        $haslo2 = $this->walidacja($haslo2);
        //zapytanie czy istnieje użytkownik o podanym loginie lub mailu
        $sqlSprawdzenieCzyIstniejeUzytkownikWBazieDanych = "SELECT * FROM uzytkownicy
            WHERE loginU = '".$login."' OR email = '".$email."'
        ";
        //wykonanie powyższego zapytania
        $rezultatSprawdzenia = $this->bazaDanych->query($sqlSprawdzenieCzyIstniejeUzytkownikWBazieDanych);
        //jezeli znaleziono użytkownika o takim  samym loginie lub emailu
        if ($rezultatSprawdzenia->num_rows > 0)
        {
            echo "<span class='zle'>Użytkownik o takim samym loginie lub email-u instnieje, Rejesrtacja nie powiodła się</span>";
            return false;
        }
        else
        {
			//sprawdzenie czy hasła są identyczne
        if ($haslo1 != $haslo2)
        {
			//jeżeli nie są identyczne hasła odpowiedni komunikat 
            echo "<span class='zle'>Hasła nie powtórzyły się</span>";
            return false;
        } 
        //zapytanie - rejestracja użytkownika - zapisanie do bazy danych
        $sqlRejestracjaUzytkownik = "INSERT INTO uzytkownicy (loginU,email,haslo) 
            VALUES ('".$login."','".$email."',SHA1('".$haslo1."'))";
            //wykonanie powyższego zapytania
        $rezultat = $this->bazaDanych->query($sqlRejestracjaUzytkownik);
        //jeżli rejestracja się powiodła odpowiedni komunikat
        if ($rezultat)
        echo "<span class='ok'>Zarejestrowano</span>";
        //jeżeli rejestracja się nie powiodła odpowiedni komunikat
        else
        echo "<span class='zle'>Rejestracja nie powiodła się</span>";
        }
        $this->rozlaczZBazaDanych();
    }
    //logowanie użytkownika
    public function zalogujUzytkownik($login,$haslo)
    {
		//walidacja
        $login = $this->walidacja($login);
        $haslo = $this->walidacja($haslo);
        //łączymy z bazą danych
        $this->polaczZBazaDanych();
        //zapytanie czy ligin i hasło się zgadzają
        $sqlLogowanie = "SELECT * FROM uzytkownicy WHERE
        loginU = '".$login."' AND haslo = SHA1('".$haslo."')";
        //wykonanie powyższego zapytania
        $rezultatLogowanie = $this->bazaDanych->query($sqlLogowanie);
        //jeżeli znaleziono dane o takim użytkowniku to logujemy
        if ($rezultatLogowanie->num_rows == 1)
        {
            $rezultatLogowanie = $rezultatLogowanie->fetch_assoc();
            //ustawienie sesji użytkownika - przechowuje login użytkownika
            $_SESSION['uzytkownik'] = $rezultatLogowanie['loginU'];
            //odpowiedzni komunikat po zalogowaniu
            echo "<span class='ok'>Zalogowano użytkownika ".$_SESSION['uzytkownik']." <a href='uzytkownik.php?akcja=wyloguj'>Wyloguj</a></span>";
        }
        else
        {
			//nie znaleziono użytkownika o takich danych - odpowiedni komunikat o tym fakcie
            echo "<span class='zle'>Nieudana próba logowania</span>";
        }//rozłączamyh z bazą danych
        $this->rozlaczZBazaDanych();
    }
    //wylogowywanie
    public function wyloguj()
    {
		//usuwanie zmiennej sesji
        unset($_SESSION['uzytkownik']);
        session_destroy();
        //odpowiedni komunikat informujący o wylogowaniu
        echo "<span class='ok'>Wylogowano</span>";
    }
    //pokazujemy info o zalogowanym użytkowniku lub o braku zalogowania 
    public function czyZalogowano()
    {
		//w zależności o istnieniu sesji użytkownika odpowiednia informacja
        if (isset($_SESSION['uzytkownik']))
        echo "<span class='ok'>Zalogowano jako ".$_SESSION['uzytkownik']."<a href='uzytkownik.php?akcja=wyloguj' >Wyloguj</a></a></span>";
        else
        echo "<span><a href='uzytkownik.php?akcja=zalogujFormularz'>Nie jesteś zalogowany - zaloguj się</a></span>";
    }
}
?>
 
[PHP] pobierz, plaintext 

Za ocenę dziękuję

Ten post edytował huberthx 25.11.2019, 21:16:51

Popełniasz standardowy błąd początkujących. Opakowałeś strukturalny kod w klasę która niewiele ma wspólnego z programowanie obiektowym. Odpowiedzialność klasy: połączenie z bazą (fatalne jak już ci pisałem wcześniej), walidacja danych, funkcje widoku (echo kodu html), zarządzanie sesją. Poza tym mamy już czasy PHP7 - gdzie jakieś typowanie? Na początek poczytaj o dependency injection, PSR. Możesz to zrobić np:

[PHP] pobierz, plaintext 
class UserEntity {
    use ArrayTrait; // metoda toArray()
 
    private $login;
    private $email;
 
    public function getLogin() : string {
        return $this->login;
    }
 
    public function setLogin(string $login) : void {
        // Niektórzy tutaj dają walidację danych chociaż nie jestem zwolennikiem
        $this->login = $login;
    }
}
 
class UserService implements UserServiceInterface {
 
    private $storage;
 
    public function __construct(\Pdo $storage) {
        $this->storage = $storage;
    }
 
    public function registerUser(UserEntity $user) : bool {
        $this->storage->beginTransaction();
        try {
            $prep = $this->storage->prepare('INSERT INTO users...');
            $prep->execute($user->toArray());
            $this->storage->commit();
            return true;
        } catch (\PDOException $e) {
             // np. logowanie zdarzenia
             $this->storage->rollBack();
             return false;
        }
    }
}
[PHP] pobierz, plaintext 

Przepraszam że nie zakładam nowego tematu ale może w tym może być.
Czy mógłby ktoś polecić jakąś dobrą książkę o OOP-ie ?

książka niewiele Ci da. Podobnie jakbyś wpisał w google "php oop" to poczytasz o strukturze klas, ewentualnie jakieś proste przykłady w dupy. Cała sztuka nie polega na tym że napiszesz Class zamiast function, tylko na zmianie w sposobie myślenia o projektowaniu kodu oraz wykorzystaniu możliwości jakie dają Tobie obiekty. Tutaj trzeba sporego doświadczenia. Na początek na pewno poczytaj o dobrych praktykach (zacznij od SOLID) i staraj sie do nich stosować. W realizacji tych praktyk pomogą Tobie wzorce projektowe (przykładowy spis: https://lukasz-socha.pl/php/wzorce-projektowe-spis-tresci/). W pewnym momencie pomyślisz o testach jednostkowych i znowu odkryjesz że Twój kod powinien wyglądać jeszcze inaczej. Wypadałoby poznac standardy kodowania (PSR). Buszuj po githubie, analizuj kody innych, zacznij korzystać z frameworków. Tak jak wspomniałem wszystko to wymaga lat praktyki, stąd też stawki jakie zarabiają starsi programiści.

ps: pamiętaj przy tym że programowanie obiektowe to tylko jeden ze sposób na rozwiązanie danego zagadnienia, nie jest wcale powiedziane że takie podejście jest zawsze właściwe a nawet pożądane

Ten post edytował gitbejbe 28.11.2019, 19:08:21

Dokładnie @gitbejbe ma rację przykłady z książek zwykle nie są życiowe, ale nie oznacza to że książka się nie przyda, teraz na HELION jest promocja 2 książki w cenie 1, więc warto np. wziąć książkę o Czystym Kodzie, RC Martin. Możesz wziąć drugą o PHP o wzorcach i obiektach, przynajmniej zakodujesz sobie podstawy, łatwiej zrozumieć także Githuba, dlaczego ludzie piszą tak a nie inaczej.

Poczytałem i oto wynik mojej nauki i prośba o ocenę.
Przepraszam za być może nieodpowiednie nazewnictwo metod bądź składowych
Oto zarys kodu:

[PHP] pobierz, plaintext 
<?php
 
class Db
{
	private static $instance;
	private $host = 'localhost';
	private $username = 'uzytkownik';
	private $password = 'haslo';
	private $database = 'bazaDanych';
	private $connection;
	public static function getInstance()
	{
		if (self::$instance === null)
		{
			self::$instance = new Db();
		}
		return self::$instance;
	}
	private function __construct()
	{
		$this->connection = new mysqli($this->host,$this->username,$this->password,$this->database);
	}
	private function __clone() {}
	public function getConnection()
	{
		return $this->connection;
	}
}
$Db = Db::getInstance();
 
class Uzytkownik
{
	private $login;
	private $haslo;
	public function setLogin($login) : void
	{
 
		$this->login = $login;
	}
	public function getLogin() : String
	{
		return $this->login;
	}
	public function setHaslo($haslo) : void
	{
		$this->haslo = $haslo;
	}
	public function getHaslo() : String
	{
		return $this->haslo;
	}
	private $db;
	public function bazaDanych($db)
	{
		$this->db = $db;
		return $this->db;
	}
	public function rejestracja($login,$haslo)
	{
		$this->setLogin($login);
		$this->setHaslo($haslo);
		$login = $this->getLogin();
		$haslo = $this->getHaslo();
		$this->db->query(/*ZAPYTANIE INSERT*/);
	}
}
 
$Uzytkownik = new Uzytkownik();
$Uzytkownik->bazaDanych($Db->getConnection());
$Uzytkownik->rejestracja('login','haslo');
 
[PHP] pobierz, plaintext 

Podstawowe pytanie - po co ci ten singleton? Kod ogólnie dalej fatalny.

Pomijając ten singleton - co ogólnie znowu zepsułem ?

np to

[PHP] pobierz, plaintext 
		$this->setLogin($login);
		$this->setHaslo($haslo);
		$login = $this->getLogin();
		$haslo = $this->getHaslo();
		$this->db->query(/*ZAPYTANIE INSERT*/);
[PHP] pobierz, plaintext 

Jaki tu sens w tym kodzie?
Na dzien dobry masz zmienne $login i haslo, potem te zmienne ustawiasz set.... a potem je pobierasz get... i w rezultacie masz to co miales
O te 4 linijki

[PHP] pobierz, plaintext 
		$this->setLogin($login);
		$this->setHaslo($haslo);
		$login = $this->getLogin();
		$haslo = $this->getHaslo();
[PHP] pobierz, plaintext 

Mozna wywalic bo nic nie wnosza do tej funkcji

Szczerze ja myślałem o tym żeby w metodach set była walidacja danych.
Przepraszam za to że nie podałem tego w kodzie - moje niedopatrzenie.