Avis code php poo

**andaman** · 18/07/2013, 17h55

Bonjour,

Etant débutant en POO, j'aimerais avoir votre avis sur le code suivant afin de savoir je peux / dois l'améliorer. Je sors du procédural donc soyez indulgent

Je crée un login / pass où je scinde la partie php et Html (2 fichiers).
Je ne poste que ce que je dois montrer.

La partie html rien de bien sorcier

Code html :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
 
<form action="index.php" name="loginform" method="post">
    <input type="text" name="login">
    <input type="password" name="password">
    <input type="submit" name="login_submit" value="Se connecter">
</form>

Ma classe de login, logout et pass perdu.
J'instance l'objet aussi dans le fichier html

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
 
class Auth{
    protected $login;
    protected $password;
    protected $email;
 
    public function login(){
        $sql = new Request();
        if(isset($_POST['login_submit'])){
            if(!empty($_POST['login']) && !empty($_POST['password'])){
                $this->login = $_POST['login'];
                $this->password = sha1($_POST['password']);
                $sql->query('SELECT * FROM users WHERE u_login = :login AND u_password = :password');
                $sql->bind(':login',$this->login);
                $sql->bind(':password',$this->password);
                $row = $sql->single();
 
                $count = $sql->rowCount();
                if($sql->rowCount() > 0){
                    $_SESSION['back_office'] = array(
                        'login' => $row->u_login,
                        'level' => $row->u_id_level
                    );
                    echo '<pre>';
                    echo 'result : '.$count.'<br>';
                    print_r($_POST);
                    print_r($_SESSION);
                    echo '</pre>';
                }else{
                    Message::ShowError(ERR_ACCOUNT_NOT_FOUND);
                }
            }else{
                Message::ShowError(ERR_ALL_FIELDS);
            }
        }
    }
 
    public function logout(){
        $_SESSION = array();
        session_unset();
        session_destroy();	
        header('Location: login');
    }
}

Merci d'avance

**m4riachi** · 18/07/2013, 18h18

Une class doit être dépendante, ne pas la relié avec des valeur qui vient de l’extérieur, comme ça si tu veux l'utiliser dans un autre projet avec un formulaire qui a des inputs différent dant l'attribut "name" tu n'aura pas a changer t'a class, le mieux c'est de passer les valeur externe en paramètres.

Un Ex mais il pourra y avoir mieux

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
 
class Auth{
	protected $login;
	protected $password;
	protected $email;
 
	public function setLogin($login)
	{
		$this->login = $login;
	}
 
	public function setPassword($password)
	{
		$this->password = $password;
	}
	public function login($table, $col_login, $col_password){
		$sql = new Request();
		$sql->query('SELECT * FROM ' . $table . ' WHERE ' . $col_login . ' = :login AND ' . $col_password . ' = :password');
		$sql->bind(':login',$this->login);
		$sql->bind(':password',$this->password);
		$row = $sql->single();
 
		$count = $sql->rowCount();
		if($sql->rowCount() > 0){
			return true;
		}else{
			return false;
		}
	}
 
	public function logout(){
		$_SESSION = array();
		session_unset();
		session_destroy();
		header('Location: login');
	}
}
 
if(isset($_POST['login_submit'])){
	if(!empty($_POST['login']) && !empty($_POST['password'])){
		$Auth = new Auth();
		$Auth->setLogin($_POST['login']);
		$Auth->setPassword($_POST['password']);
		if ($Auth->login('users', 'u_login', 'u_password'))
		{
			$_SESSION['back_office'] = array(
					'login' => $row->u_login,
					'level' => $row->u_id_level
			);
			echo '<pre>';
			echo 'result : '.$count.'<br>';
			print_r($_POST);
			print_r($_SESSION);
			echo '</pre>';
		}
		else
		{
			Message::ShowError(ERR_ACCOUNT_NOT_FOUND);
		}
	}
}
else
{
	Message::ShowError(ERR_ALL_FIELDS);
}

**andaman** · 18/07/2013, 18h40

Merci beaucoup de cette réponse clair et nette

En fait, dans ma class Request + ma class connecion dans un autre fichier , j'ai ceci:

Code php :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
 
class Request{
    protected $db;
    protected $stmt;
 
    public function __construct(){
        $this->db = DbConnect::init();
    }
    public function query($query){
        $this->stmt = $this->db->prepare($query);
        return $this->stmt;
    }
    public function execute(){
        return $this->stmt->execute();
    }
    public function rowCount(){
        return $this->stmt->rowCount();
    }
    public function single(){
        $this->execute();
        return $this->stmt->fetch(PDO::FETCH_OBJ);
    }
    public function bind($param, $value, $type = null){
        if(is_null($type)){
            switch(true){
                case is_int($value):
                    $type = PDO::PARAM_INT;
                    break;
                case is_bool($value):
                    $type = PDO::PARAM_BOOL;
                    break;
                case is_null($value):
                    $type = PDO::PARAM_NULL;
                    break;
                default:
                    $type = PDO::PARAM_STR;
            }
        }
        $this->stmt->bindValue($param, $value, $type);
    }
}
 
 
class DbConnect{
    private static $db;
 
    public static function init(){
        if(!self::$db){
            try{
                $dsn = 'mysql:host='.DB_HOST.';dbname='.DB_NAME.';charset=UTF-8';
                self::$db = new PDO($dsn, DB_USER, DB_PASS);
                self::$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING);
                self::$db->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_OBJ);
                self::$db->setAttribute(PDO::MYSQL_ATTR_INIT_COMMAND,'SET NAMES utf8');
//                echo 'Connexion OK';
            }catch(PDOException $e) {
                die('Connection error: ' . $e->getMessage());
            }
        }
        return self::$db;
    }
}

Est ce que cela est correct ?
Merci davance

**comode** · 18/07/2013, 19h29

Disons que c'est pas terrible comme façon de faire...

Dans ton cas, à chaque requête, tu vas de voir ouvrir et initialiser une connexion avec la BDD. Or, pour des raisons qu'on ne détaillera pas ici, il est quand même très préférable d'ouvrir la connexion une seule fois, de faire ses requetes puis la fermer.

La meilleur façon de faire ça est d'utilisé un design pattern appelé "Singleton". C'est pas loin d'être ce que tu as fait, si ce n'est que l'handle de connexion doit être stocké dans ta class static dbconnect (que je te conseil par ailleurs de dériver de PDO). En plus de la variable static qui contiendra ta connexion, tu dois y mettre une méthode static genre GetInstance() qui va initialiser la connexion si elle n'existe pas encore, et retourner le contenu de la variable static qui stock la connexion.

De la sorte, de n'importe où dans le code, tu pourras directement utiliser les méthodes de la PDO (qui est deja une classe, pas la peine de faire une class Request qui l'appel) depuis n'importe ou dans le code en faisant :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
$pdo_statement = DbConnect::getInstance()->prepare('select * from user');
$pdo_statement->execute() etc...

**andaman** · 18/07/2013, 19h35

MERCI

Je sens que j'avance !

Mais commode Que veux tu dire par :

que je te conseil par ailleurs de dériver de PDO

un design pattern appelé "Singleton", j'ai vu mais c'est pas très clair pour moi...
Si je comprends bien avec un singleton, j'instancie ma connexion et puis fini ?

Si oui? COmment? Peut etre un exemple?

Effectivement je pense que ma classe Request je vais la virer, elle fait "fantaisie"

Merci de vos réponses

**comode** · 18/07/2013, 19h49

Envoyé par andaman

Si je comprends bien avec un singleton, j'instancie ma connexion et puis fini ?

Oui, après, tu as au final ta connexion ouverte accessible via une méthode static, donc utilisable depuis n'importe où sans avoir besoin de faire un new sur l'objet. C'est un peu comme une variable globale quoi :p

Si oui? COmment? Peut etre un exemple?

Comme je disais, on dérive notre class de PDO de façon à en hériter toutes les méthodes :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
class SPDO extends PDO // Signleton PDO
{
	// Instance partagée par le singleton
	private static $instance = false;
 
	// Envoie l'instance partagée depuis n'importe où dans le code
	public static function getInstance()
	{
		if(!self::$instance)
			throw new Exception("PDO instance  not initialized");// on oblige une connexion explicite dans le code de l'application.
		else
			return self::$instance; // on retourne le handle de la connexion
	}
 
	/* CONNECTION A LA BASE DE DONNEES
		$dsn => nom et host de la bdd, a définir comme suit : 'mysql:dbname='.BDD_NAME.';host='.BDD_CONNECT.''
		$username => nom d'utilisateur de la BDD
		$password => password de la bdd
		$driver_options => tableau de parametres
	*/
	public static function connect($dsn, $username = "root", $password = "", array $driver_options = NULL)
	{
		self::$instance = new MyPDO($dsn, $username, $password, $driver_options);
		self::$instance->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION ); // mode exception
		self::$instance->setAttribute( PDO::MYSQL_ATTR_USE_BUFFERED_QUERY, true ); // use buffer (autorise plusieurs requetes en même temps)
		self::$instance->setAttribute( PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC ); //sauf avis contraire, on fait un FETCH_MODE pour avoir uniqumeent un tableau associatif clef => vlaur
		return self::$instance;
	}
}

Une fois que la class est déclarée comme ça, il faut l'initialiser une fois en début de script (ou le faire automatiquement a l'appel de getInstence() si tu préfères)

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
$pdo = SPDO::connect('mysql:dbname='.BDD_NAME.';host='.BDD_HOST.'', BDD_USER, BDD_PASS, array(						PDO::MYSQL_ATTR_USE_BUFFERED_QUERY => true
						));

et après, bha tu peux utiliser toutes fonctions de la PDO (preque) normalement depuis n'importe ou dans le code sans re-ouvrir la connexion :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
class machin
{
     public function __contruct($id)
    {
        $s = SPDO::getInstance()->prepare('select * from machin where id = :id');
        $s->execute(array(':id' => $id));
        foreach($s as $res)
             print_r($res);
       $s->closeCursor();
    }
}

Pour le code, je l'ai extraint d'un de mes projets, donc a toi de mettre des parametres, des variables et des constantes qui vont bien

**andaman** · 18/07/2013, 19h54

Merci de ta réactivité.

Deernière question pour aujourd'hui car il est 1h du matin cic.

Quand tu déclares ta classe comme ceci:

class SPDO extends MyPDO

Je comprends que tu peux utiliser MyPDO maiss MyPDO vient de où?

Désolé mais la POO c'est tout neuf pour moi

Merci d'avance

**comode** · 18/07/2013, 19h57

J'ai corrigé pendant que tu rédigeais ta réponse ! Comme je disais, c'est un code qui vient d'un projet à moi et que j'ai simplifié à la volée pour qu'il parle ici.

MyPDO était une class de ce projet (qui dérivait elle mêmle de PDO) et j'ai oublié de l'enlever :p

donc il fallait bien lire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part