Avis code php poo

Version imprimable

Avis code php poo

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 :P

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:

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:

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

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:

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);
}

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:

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

18/07/2013, 19h29
comode
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:

1 2 $pdo_statement = DbConnect::getInstance()->prepare('select * from user'); $pdo_statement->execute() etc...
18/07/2013, 19h35
andaman

MERCI ;)
Je sens que j'avance !

Mais commode Que veux tu dire par :

Citation:

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

Citation:

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

Citation:

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:

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:

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:

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

18/07/2013, 19h54
andaman

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
18/07/2013, 19h57
comode

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:

class SPDO extends PDO

et non

Code:

class SPDO extends MyPDO
18/07/2013, 19h59
andaman

Merci 8-) M'en doutais mais je préférais poser la question. Je regarde tout cela demain tranquillement.

En tt les cas, un grand merci