- Nieprzestrzeganie standardu PSR-2 (wygląd kodu) w zasadzie w żadnej kwestii. Choć nie jest to oficjalny standard, jest tak popularny, że jego przestrzeganie dla wielu to oczywistość i podstawa.
- Nazewnictwo: widzę jeden plik "registration" a drugi "zalogowany". To w końcu w jakim języku piszesz i nazywasz? Powinien to być wszędzie angielski.
- Wszystkie klasy dziedziczące po jednej, która oferuje połączenie do bazy to raczej słaby sposób. Jak w pewnych miejscach by to przeszło, tak np. gdy jedna klasa dodaje użytkownika, waliduje, sprawdza wiek i inne rzeczy, to już za dużo.
- Wszędzie brak typów przekazywanych argumentów, brak też typów zwracanych. PHP 7 nie jest już nowością, raczej stosowanie tego powinno być normą jeśli tylko jest możliwe.
- Nawiązywanie połączenia z bazą: gdybym stworzył kilka razy obiekt (albo kilka różnych obiektów dziedziczących po klasie abstrakcyjnej) to połączenie z bazą nawiązałoby się kilkukrotnie, moim zdaniem bez potrzeby.
- Konfiguracja, np. bazy danych:
$connect = new mysqli('localhost', 'root', '', 'shop');
Dane dostępowe wypadałoby gdzieś wyprowadzić, nikt nie szuka danych dostępowych w klasach w środku kodu.
-
Znacznik <font> jest zdeprecjonowany (a więc nie należy go używać, a nadawać style przez CSS):
echo "<font color=red>".$_SESSION['error']."</font></br>";
Ponadto nie ma zapisu </br>, jest <br> albo ewentualnie <br/>. Nie widzę też sensu używania cudzysłowów do tekstu, w którym nic nie jest podstawiane (nie zawiera w sobie żadnej zmiennej). Albo zamieniłbym to w taki sposób, aby nie robić konkatenacji z tą zmienną sesyjną, albo zamienił cudzysłowy na apostrofy.
-
Inputy muszą mieć labele.
<input type="text" name="nick" placeholder="nick"></br>
-
require_once "db_construct.php";
Takie dołączanie działa, ale nikt już tego nie używa w czasach Compoera i autoloadingu przez PSR-4. Oczywiście jeśli to jest jedna klasa to może i ten Composer nie ma sensu, ale jak dla mnie klas tu powinno być więcej i przy okazji poznałbyś coś, co jest standardem.
- addslashes() to nie jest zabezpieczenie przed SQL injection, a więc cała ta klasa bez bindowania jest do niczego.
$nick = addslashes($nick);
- md5() nie nadaje się do hashowania haseł, używamy password_hash() i password_verify().
-
if($this::nick_password($nick, $password)){
Albo $this-> albo self:: - pierwsze dla zwykłych metod, drugie dla statycznych.
-
//jeśli wszystko w porządku zalogowanie użytkownika
Takich komentarzy raczej należy się oduczać, po pierwsze są po polsku, po drugie kod powinien być napisany tak, aby dało się to samo z niego zobaczyć.
-
$login->show($_POST['nick'], $_POST['password']);
A jak ktoś nie prześle tych danych to co? Będą ostrzeżenia, gdyż w tablicy $_POST nie będzie takich elementów. Albo sprawdź czy istnieją i dopiero odczytuj, albo użyj np. filter_input (samo dba o sprawdzenie i odczytanie, do tego można dorzucić jakieś filtry czy walidację).
-
if(strlen($nick) == 0)
strlen() ma problem z polskimi znakami, użyj mb_strlen().
-
if(!strpos($email, '@') || !strpos($email, '.'))
Coś kiepska ta walidacja emaila, "@." jest dla Ciebie poprawnym emailem?
-
$date_Y = date('Y')-$date_explode[0];
Operować na datach znacznie wygodniej jest na klasie DateTime.
-
$what = "'$this->nick', '$this->password', '$this->email', '$this->date', 0";
A tu można zrobić śliczne SQL injection, np. przy emailu nie ma nawet tego addslashes().
-
W samej bazie złe kodowanie, masz jakieś latin1.
No cóż, sporo do poprawy, w zasadzie napisania od nowa. Plus mogę dać za próbę obiektowości, a nie robienia tego strukturalnie i za Gita/GitHuba. Mam nadzieję, że moja ocena będzie Ci pomocna. Nikt nie wymaga od osoby zaczynającej naukę, aby to wszystko umiała od razu, tak więc mam nadzieję, że podłapiesz odpowiednie tematy i będzie tylko lepiej.
