Hallo,

Gister weer begonnen met OOP en ik had gister een login/register/passforget systeem gemaakt met OOP en ik vroeg mij af of ik het zo ongeveer goed deed en wat ik kan verbeteren.

De database users ziet er zo uit:


databaseClass.php

<?php
    class Database
    {
        private $servername;
        private $username;
        private $password;
        private $database;
        private $conn;

        protected function connect()
        {
            if ($this->conn == NULL)
            {
                $this->servername = 'localhost';
                $this->username = 'root';
                $this->password = '';
                $this->database = 'oop';

                $this->conn = new mysqli($this->servername, $this->username, $this->password, $this->database);
            }
            
            return $this->conn;
        }
    }
?>


userClass.php

<?php
    class User extends Database
    {
        protected function generateRandomCode($length)
        {
            $unique = false;

            while (!$unique)
            {
                $characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
                $charactersLength = strlen($characters);
                $randomCode = '';

                for ($i = 0; $i < $length; $i++)
                {
                    $randomCode .= $characters[rand(0, $charactersLength - 1)];
                }

                $sql = "SELECT `pcode` FROM `users` WHERE `change_ww_code` = '" . $this->connect()->real_escape_string($randomCode) . "'";
                $query = $this->connect()->query($sql);

                if ($query || $query->num_rows == 0)
                {
                    $unique = true;
                }
            }

            if ($unique == true)
            {
                return $randomCode;
            }
        }

        public function login($username, $password)
        {
            $sql = "SELECT `username`, `password`, `active` FROM `users` WHERE `username` = '" . $this->connect()->real_escape_string($username) . "'";
            $query = $this->connect()->query($sql);

            if ($query->num_rows == 0)
            {
                throw new Exception("De inlog gegevens zijn onjuist.");
            }
            else
            {
                $data = $query->fetch_assoc();

                if ($data['active'] == 0)
                {
                    throw new Exception("Je account is gedeactiveerd, neem contact op met een administrator.");
                }
                elseif (password_verify($password, $data['password']) == true)
                {
                    $sql = "UPDATE `users` SET `password` = '" . password_hash($password, PASSWORD_BCRYPT) . "', `last_online` = '" . date('Y-m-d H:i:s') . "', `change_ww_code` = '', `change_ww_perm` = '0' WHERE `username` = '" . $this->connect()->real_escape_string($username) . "'";
                    $query = $this->connect()->query($sql);
                }
                else
                {
                    throw new Exception("De inlog gegevens zijn onjuist.");
                }
            }
        }

        public function register($username, $password, $passwordrepeat)
        {
            $sql = "SELECT `username`, `password` FROM `users` WHERE `username` = '" . $this->connect()->real_escape_string($username) . "'";
            $query = $this->connect()->query($sql);

            if ($query->num_rows == 1)
            {
                throw new Exception("De gebruikersnaam is ongeldig.");
            }
            elseif ($password != $passwordrepeat || strlen($password) <= 5)
            {
                throw new Exception("Controleer de wachtwoorden of ze overeenkomen en 6 tekens of langer zijn.");
            }
            else
            {
                $sql = "INSERT INTO `users` (`username`, `password`, `register_date`) VALUE ('" . $this->connect()->real_escape_string($username) . "', '" . password_hash($password, PASSWORD_BCRYPT) . "', '" . date('Y-m-d H:i:s') . "')";
                $query = $this->connect()->query($sql);
                if (!$query)
                {
                    throw new Exception("Er is iets fout gegaan, neem contact op met een administrator.");
                }
            }
        }

        public function passForget($username, $password, $passwordrepeat, $code)
        {
            $sql = "SELECT `username`, `change_ww_perm`, `change_ww_code` FROM `users` WHERE `username` = '" . $this->connect()->real_escape_string($username) . "'";
            $query = $this->connect()->query($sql);

            if ($query->num_rows == 0)
            {
                throw new Exception("De gebruikersnaam is ongeldig.");
            }
            else
            {
                $data = $query->fetch_assoc();
                if ($password != $passwordrepeat || strlen($password) <= 5)
                {
                    throw new Exception("Controleer de wachtwoorden of ze overeenkomen en 6 tekens of langer zijn.");
                }
                elseif ($data['change_ww_perm'] == 0)
                {
                    throw new Exception("Je hebt geen toestemming om je wachtwoord te veranderen.");
                }
                elseif (!isset($code) || $code != $data['change_ww_code'])
                {
                    throw new Exception("De opgegeven code is onjuist.");
                }
                else
                {
                    $sql = "UPDATE `users` SET `password` = '" . password_hash($password, PASSWORD_BCRYPT) . "', `change_ww_perm` = '', `change_ww_code` = '' WHERE `username` = '" . $this->connect()->real_escape_string($username) . "'";
                    $query = $this->connect()->query($sql);
                    if (!$query)
                    {
                        throw new Exception("Er is iets fout gegaan, neem contact op met een administrator.");
                    }
                }
            }
        }
    }
?>


Ik weet niet of ik teveel if/elseif/else statements heb zo ja, zouden jullie mij dan kunnen vertellen hoe ik dit kan verbeteren :)

Graag hoor ik verbeter punten.

Mvg,
Rob
In reactie op Thomas,

Abstractie is natuurlijk zeker niet het doel van programmeren. Flexibiliteit daar in tegen wel. Hoe flexibeler je code hoe vaker je je code kunt hergebruiken.

>> hoe vaak zal er in de levensloop van een applicatie (vrij) geschakeld worden tussen verschillende database-typen?
(Bijna) niet. Maar wanneer je je code wilt hergebruiken voor een andere klant of een ander project dan is de kans wel aanwezig dat je met een andere database-type te maken krijgt.

Er zijn met behulp van Composer veel goede standaard libraries te vinden. Deze kunnen dan later (ook wanneer je project al lang afgerond is) ge-update worden. En dat kan zeker wel noodzakelijk zijn. Zo zijn er in het verleden PHP functies als verouderd of onveilig bestempeld. (deprecated). Een recent voorbeeld zijn de mysql_ functies die uit PHP versie 7 verbannen zijn. Wie weet komt er in de toekomst een vervanger voor mysqli of voor de password hashing functies. Stel dat dit gebeurt en de server waar jouw code op draait wordt voorzien van de nieuwste PHP versie zonder de oude deprecated functies dan ben je een blij persoon als enkel maar de updates hoeft op te halen van de libraries die je gebruikt.

Als je heel erg strict wilt gaan worden dan moet je eigenlijk in je bovenste laag van je code helemaal geen standaard PHP functie meer aanroepen. In plaats daarvan roep je een method in een van je libraries aan. Mochten de makers van PHP een functie weglaten of wijzigen of iets dergelijks dan hoef je alleen maar een library aan te passen en/of te updaten.
Frank Nietbelangrijk op 05/01/2018 18:17:19

Abstractie is natuurlijk zeker niet het doel van programmeren. Flexibiliteit daar in tegen wel. Hoe flexibeler je code hoe vaker je je code kunt hergebruiken.

Maar iets dat heel abstract is, is juist nauwelijks concreet en daarmee dus universeler inzetbaar.

Met andere woorden: abstractie en flexibiliteit sluiten elkaar niet uit, maar versterken elkaar juist.

(Het praktische probleem is meer dat mensen het erg moeilijk vinden om de vertaalslag te maken van een universele abstractie naar hun onderhanden, concreet probleem. Maar dat is dan vooral hun probleem eigenlijk.)

Reageren