Débutant : Classe membre bien écrite ?

Eléphant du PHP | 386 Messages

02 janv. 2017, 06:00

Salut,

je débute la POO, je voulais savoir si mon début de classe était bien écrite ?
Je pense qu'il manque les setters pour vérifier et sécuriser l'affichage des variables ?

Tout fonctionne quand même, et je préfère l'organisation de mon site en POO plutôt qu'en procédural (même pour un accès membre) :wink:

Code : Tout sélectionner

<?php class Member { private $id; private $username; private $email; private $surname; private $name; private $sex; private $day; private $month; private $year; private $userstart; private $userdate; public function __construct($db, $username){ $user = $db->query('SELECT * FROM users WHERE username = :username',['username' => $username])->fetch(); $this->id = $user->id; $this->username = $user->username; $this->email = $user->email; $this->surname = $user->surname; $this->name = $user->name; $this->sex = $user->sex; $this->day = $user->day; $this->month = $user->month; $this->year = $user->year; $this->userstart = $user->userstart; $this->userdate = $user->userdate; } public function getId(){ return $this->id; } public function getUsername(){ return $this->username; } public function getEmail(){ return $this->email; } public function getSurname(){ return $this->surname; } public function getName(){ return $this->name; } public function getSex(){ return $this->sex; } public function getDay(){ return $this->day; } public function getMonth(){ return $this->month; } public function getYear(){ return $this->year; } public function getUserstart(){ return $this->userstart; } public function getUserdate(){ return $this->userdate; } }
Sinon, avec quelques explications, merci beaucoup =)

Avatar de l’utilisateur
Administrateur PHPfrance
Administrateur PHPfrance | 13233 Messages

02 janv. 2017, 07:04

Tout dépend quel niveau de POO tu veux atteindre, on peux dire oui, ou non ^^

Dans le plus pur esprit Object, un objet a une seule utilité, ta classe Member devrait donc juste être une représentation de ton membre, et ne devrait pas contenir la logique SQL.
On remarque ce soucis quand on fait ce qu'on appelle de l'injection de dépendance : ta classe Member a besoin d'une instance $db pour fonctionner.

Une classe doit prendre dans le constructeur tout les champs "obligatoires" et proposer des setter pour les autres champs. Ces setter doivent vérifier qu'on ne passe pas n'importe quoi, pour que le but de cette classe est de représenter le membre.

Stocker une date au format jour, mois et année, je te conseille de remplacer par une vrai date, ce qui te permettra des calculs bien plus simple par la suite

Je n'oublierais pas les PHPDoc pour aider les personnes qui lisent ton code (et donc, toi dans le futur), à comprendre ce qui était prévu ici.

Je remplacerais les "private" par des protected pour permettre la surcharge future

Voici un exemple de ce que j'aurais fait de ta classe (je suis parti du postulat que l'id et le username étaient obligatoires) :
<?php

class Member
{
    /**
     * @var string
     */
    protected $id;

    /**
     * @var string
     */
    protected $username;

    /**
     * @var string
     */
    protected $email;

    /**
     * @var string
     */
    protected $surname;

    /**
     * @var string
     */
    protected $name;

    /**
     * @var string
     */
    protected $sex;

    /**
     * @var Datetime
     */
    protected $birthdate;

    /**
     * @var string
     */
    protected $userstart;

    /**
     * @var Datetime
     */
    protected $userdate;

    /**
     * Allowes values for $sex
     * @var array
     */
    static protected $allowed_sex = ['M', 'F'];

    /**
     * Member constructor.
     *
     * @param string $id
     * @param string $username
     *
     * @return static
     */
    public function __construct($id, $username)
    {
        $this->id = $id;
        $this->username = $username;
    }

    /**
     * @return string
     */
    public function getId(): string
    {
        return $this->id;
    }

    /**
     * @param string $id
     */
    public function setId(string $id)
    {
        $this->id = $id;
    }

    /**
     * @return string
     */
    public function getUsername(): string
    {
        return $this->username;
    }

    /**
     * @param string $username
     */
    public function setUsername(string $username)
    {
        $this->username = $username;
    }

    /**
     * @return string
     */
    public function getEmail(): string
    {
        return $this->email;
    }

    /**
     * @param string $email
     */
    public function setEmail(string $email)
    {
        if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
            throw new \InvalidArgumentException("'$email' is not a valid email adress");
        }

        $this->email = $email;
    }

    /**
     * @return string
     */
    public function getSurname(): string
    {
        return $this->surname;
    }

    /**
     * @param string $surname
     */
    public function setSurname(string $surname)
    {
        $this->surname = $surname;
    }

    /**
     * @return string
     */
    public function getName(): string
    {
        return $this->name;
    }

    /**
     * @param string $name
     */
    public function setName(string $name)
    {
        $this->name = $name;
    }

    /**
     * @return string
     */
    public function getSex(): string
    {
        return $this->sex;
    }

    /**
     * @param string $sex
     */
    public function setSex(string $sex)
    {
        if( !in_array($sex, static::$allowed_sex) ) {
            throw new \InvalidArgumentException("'$sex' is not a valid value (allowed: ".implode(',', static::$allowed_sex).")");
        }

        $this->sex = $sex;
    }

    /**
     * @return Datetime
     */
    public function getBirthdate(): Datetime
    {
        return $this->birthdate;
    }

    /**
     * @param Datetime $birthdate
     */
    public function setBirthdate(Datetime $birthdate)
    {
        $this->birthdate = $birthdate;
    }

    /**
     * @return string
     */
    public function getUserstart(): string
    {
        return $this->userstart;
    }

    /**
     * @param string $userstart
     */
    public function setUserstart(string $userstart)
    {
        $this->userstart = $userstart;
    }

    /**
     * @return Datetime
     */
    public function getUserdate(): Datetime
    {
        return $this->userdate;
    }

    /**
     * @param Datetime $userdate
     */
    public function setUserdate(Datetime $userdate)
    {
        $this->userdate = $userdate;
    }
}
Après, je pense que tu poseras la question "mais où est-ce que je met la requête SQL qui charge cet objet ?" et la réponse est "dans un nouvel objet dont la responsabilité est d'hydrater ton objet Member et de te le retourner.
Cette classe là ayant pour responsabilité d'aller chercher les données du membre dans la bdd et de retourner un objet Member chargé, il est normal qu'elle ai une dépendance sur PDO (ou autre)

Mais ce que tu tentes de faire est déjà très courant en PHP avec les ORM (Doctrine, Propel)
Connaître son ignorance est la meilleure part de la connaissance
Pour un code lisible : n'hésitez pas à sauter des lignes et indenter

twitter - site perso - Github - Zend Certified Engineer

Eléphant du PHP | 386 Messages

02 janv. 2017, 17:05

Merci pour ta réponse,

- Mais si je fais déjà les vérifications à l'inscription pour les setters, il n'y en a pas besoin dans la partie membre ?
- Je comprend le setter pour vérifier l'email mais pour le sexe, je ne comprend pas du tout la condition dans le setter et pourquoi tu le passe en static ?
- Pour la date de naissance, à l'inscription, j'utilise des selects, comment je peux faire pour réunir 3 variables (jour, mois, année) en une variable qui donnerai en base de donnée : 18/01/1984 ?

- Et donc ma requête je la place en haut de mon fichier php ?
J'ai fais ça pour mon fichier index : (Je précise que j'ai une classe Authentification qui gère le login, l'inscription, et qui met dans la variable de session "auth" toutes les infos du membre.

Code : Tout sélectionner

require '../inc/bootstrap.inc.php'; // Autoloader App::getAuth()->restrict(); // Page restreinte $db = App::getDatabase(); // Connexion bdd $user = new Member($_SESSION['auth']->id,$_SESSION['auth']->username); // Nouveau membre connecté $user = $db->query('SELECT * FROM users WHERE username = :username', ['username' => $username])->fetch(); // On récupère les infos du membre en question
Mercii =)
Dernière édition par nico44530 le 02 janv. 2017, 17:17, édité 2 fois.

Avatar de l’utilisateur
Administrateur PHPfrance
Administrateur PHPfrance | 13233 Messages

02 janv. 2017, 17:14

Les vérifications à faire dans la classe Member concerne la validité du format des données, pas que c'est un membre valide.
Il faudra encore faire des vérifications pour valider que le membre existe en BDD avant de le charger, mais tu devrais le faire dans une autre classe.

Le check que je propose dans le setter setSex, c'est pour te donner un exemple de validation du format des données.
Ce check valide que la valeur du champ "sex" de la classe ne peux être que "M" ou "F".
Je suis passé par un attribut de classe, parce que tu pourrais décider d'étendre ta classe pour permettre le genre "Undetermined", et il te suffirais alors de faire :
class MemberExtended extend Member 
{
    static protected $allowed_sex = ["M", "F", "U"] 
}
Et tout le reste du code fonctionnerais.

Pour ta requête, tu devrais créer une autre classe, par exemple MemberLoader, qui ferais les requêtes et qui créerais une instance de Member
Connaître son ignorance est la meilleure part de la connaissance
Pour un code lisible : n'hésitez pas à sauter des lignes et indenter

twitter - site perso - Github - Zend Certified Engineer

Eléphant du PHP | 386 Messages

02 janv. 2017, 17:30

Merci pour l'explication ;)

Donc pour la classe MemberLoad le constructeur aurait la base de donnée en paramètre et elle hériterai de la classe membre pour les infos ?
J'aurai juste à faire des fonctions pour récupérer les infos du membre :

Code : Tout sélectionner

public function getInfo($info){ // $info est la variable qu'on veut trouver $user = $db->query('SELECT * FROM users WHERE username = :username',['username' => $username])->fetch(); return $this->user->$info; }
Ou peux-être que c'est pas comme ça qu'il faut faire

Eléphant du PHP | 386 Messages

02 janv. 2017, 17:48

J'ai une erreur : Parse error: syntax error, unexpected ':', expecting ';' or '{' in /var/www/class/Member.class.php on line 29

public function getId(): string

Avatar de l’utilisateur
Administrateur PHPfrance
Administrateur PHPfrance | 13233 Messages

03 janv. 2017, 00:52

Quelle est ta version de PHP ?
Connaître son ignorance est la meilleure part de la connaissance
Pour un code lisible : n'hésitez pas à sauter des lignes et indenter

twitter - site perso - Github - Zend Certified Engineer

Eléphant du PHP | 386 Messages

03 janv. 2017, 01:36

Version 5.6

Avatar de l’utilisateur
Administrateur PHPfrance
Administrateur PHPfrance | 13233 Messages

03 janv. 2017, 03:51

J'ai utilisé des kewords PHP 7.
Est-ce que tu sais que PHP 5.6 est en "security fixes only" ? (http://php.net/supported-versions.php)
Si tu as le choix, il faudrait que tu fasses la mise à jour rapidement.
Connaître son ignorance est la meilleure part de la connaissance
Pour un code lisible : n'hésitez pas à sauter des lignes et indenter

twitter - site perso - Github - Zend Certified Engineer

Avatar de l’utilisateur
Modérateur PHPfrance
Modérateur PHPfrance | 8675 Messages

03 janv. 2017, 13:50

salut,

juste en complément : recherche des info sur le modèle MVC (modèle, vue, contrôleur) qui est le modèle le plus courant pour séparer les couches.
ajoute à cela la couche "DAO" (Data Accès Object") et tu va savoir ce que tu vas pouvoir faire de ta requête SQL.

@+
Il en faut peu pour être heureux ......

Avatar de l’utilisateur
Administrateur PHPfrance
Administrateur PHPfrance | 13233 Messages

03 janv. 2017, 16:58

@moogli je voulais y aller un peu plus didactique, mais oui, je l'emportais sur ce chemin ^^
Connaître son ignorance est la meilleure part de la connaissance
Pour un code lisible : n'hésitez pas à sauter des lignes et indenter

twitter - site perso - Github - Zend Certified Engineer

Avatar de l’utilisateur
Modérateur PHPfrance
Modérateur PHPfrance | 8675 Messages

04 janv. 2017, 09:57

didaquoi ? :mrgreen:
Il en faut peu pour être heureux ......

Eléphant du PHP | 386 Messages

05 janv. 2017, 01:48

Mon serveur est à jour avec php 7 :wink: (Donc ça fonctionne bien pour les vérifications de types de variables)

Le modèle MVC est fait pour simplifier la vie des développeurs.
Mais moi je trouve que je gère sans MVC l'architecture de mon code, je suis seul à le connaître, surtout pour un projet pas si énorme que ça.
Si je n'aime pas ce genre d'architecture pour mon site (même si c'est trrèèès pratique pour tous), je ne vais pas me forcer à l'utiliser, et donc faire les choses alternativement ;-)

Avatar de l’utilisateur
Administrateur PHPfrance
Administrateur PHPfrance | 13233 Messages

05 janv. 2017, 04:40

Le MVC est le standard actuel. Si tu ne l'utilise pas, c'est que tu fais un développement alternatif ;)

Sinon, tu veux apprendre à faire de la POO, tu vas au devant de pleins de questions qui sont déjà répondus par des frameworks.

Sinon, tu en es où maintenant sur ta classe ?
Connaître son ignorance est la meilleure part de la connaissance
Pour un code lisible : n'hésitez pas à sauter des lignes et indenter

twitter - site perso - Github - Zend Certified Engineer

Eléphant du PHP | 386 Messages

05 janv. 2017, 16:17

Salut,

Beh je préfère avoir un développement alternatif, tout en faisant de la POO, c'est possible vu que ma classe fonctionne, mais juste mal écrite.

J'aimerai que la requête dans le constructeur sois ailleurs pour trouver les infos des membres, donc faire une méthode qui ferai ça, et appeler cette méthode dans le constructeur