+- JeuWeb - Crée ton jeu par navigateur (https://jeuweb.org)
+-- Forum : Discussions, Aide, Ressources... (https://jeuweb.org/forumdisplay.php?fid=38)
+--- Forum : Programmation, infrastructure (https://jeuweb.org/forumdisplay.php?fid=51)
+--- Sujet : [Résolu] Améliorations possibles et conseils (/showthread.php?tid=4988)
Pages :
1
2
[Résolu] Améliorations possibles et conseils - Joojo - 13-07-2010
Bonjour avec les vacances je me suis remit dans le développement web. J'ai un nouveau projet de jeu tête (qui ne verra jamais le jour) mais cette fois j'utilise la POO de PHP, la PDO et enfin j'aimerai respecter (un peu) le modèle MVC.
J'ai donc commencer par un script tout bête, l'inscription du membre. Il est tout simple et fonctionne. Mais n'étant pas encore très à l'aide avec la "conceptualisation" que demande la POO ainsi que ses diverses fonctions, je voudrai savoir si il y a des choses qui vous pics les yeux ou qui pourraient être améliorées, simplifiées. Bref je recherche des critiques.
Pour ce script je n'ai pas utilisé la modèle MVC, la flemme de créer une page modèle pour 2 requêtes sql (je commence bien
iffle: ). J'ai donc la page Inscription.php ou est le formulaire et enfin le fichier Inscription.class.php.Voici Inscription.php
Code PHP :
<?php
function chargerClasse($classname)
{
if(is_file('controleurs/'.$classname.'.class.php'))
require 'controleurs/'.$classname.'.class.php';
elseif(is_file('modeles/'.$classname.'.class.php'))
require 'modeles/'.$classname.'.class.php';
else
echo 'La classe demandée n\'existe pas.';
}
spl_autoload_register('chargerClasse');
session_start();
include('class/bdd.class.php');
if(isset($_POST['login_membre'], $_POST['pass_membre'], $_POST['pass_membre1'], $_POST['mail_membre'], $_POST['sexe']))
{
if ($_POST['login_membre'] != NULL AND $_POST['pass_membre'] != NULL AND $_POST['pass_membre1'] != NULL AND $_POST['mail_membre'] != NULL AND $_POST['sexe'] != NULL)
{
$inscription = new Inscription($_POST, $bdd);
$erreur = $inscription->verifierDonnees();
if($erreur == FALSE)
{
$inscription->enregistrerInscription();
$inscription->envoyerConfirmationMail();
$erreur = "Vous vous êtes bien inscrits sur Lisla.";
}
}
}
Plus loin dans index.php est inclut le menu (menu.php) qui contient:
Code PHP :
<?php
if(isset($erreur))
echo $erreur;
Et enfin Inscription.class.php
Code PHP :
<?php
class Inscription
{
//Les membres de la classe
private $login;
private $password;
private $password1;
private $email;
private $email1;
private $sexe;
private $bdd;
const TYPE_MEMBRE = 1; //Membre (2 modérateur, 3 admin)
public function __construct($tableau, $bdd)
{
$this->login = trim($tableau['login_membre']);
$this->password = $tableau['pass_membre'];
$this->password1 = $tableau['pass_membre1'];
$this->email = $tableau['mail_membre'];
$this->sexe = $tableau['sexe'];
$this->bdd = $bdd;
}
public function verifierDonnees()
{
$utilisation_pseudo = $this->bdd->prepare('SELECT COUNT(id) AS nbre_entrees FROM membres WHERE login = ?')or die(print_r($bdd->errorInfo()));
$utilisation_pseudo->execute(array($this->login));
$resultats = $utilisation_pseudo->fetch(PDO::FETCH_OBJ);
$utilisation_pseudo->closeCursor();
if($resultats->nbre_entrees < 1)
{
if(strlen($this->password) > 6)
{
if($this->password == $this->password1)
{
if (preg_match("!^[a-z0-9._-]+@[a-z0-9._-]{2,}\.[a-z]{2,4}$!", $this->email))
{
if($this->sexe == "F" OR $this->sexe == "M")
{
return FALSE;
}
else
return '';
}
else
return 'Votre adresse mail n\'est pas valide.';
}
else
return 'Vos mots de passe sont différents.';
}
else
return 'Votre mot de passe doit faire plus de 6 charactères.';
}
else
return 'Ce pseudo est déjà utilisé.';
}
public function enregistrerInscription()
{
$this->password = sha1($this->password);
$membres = $this->bdd -> prepare('INSERT INTO membres (id, login, password, email, type, adresse_ip)
VALUES ("", :login, :password, :email, :type, :adresse_ip)')or die(print_r($bdd->errorInfo()));
$membres -> execute(array('login' => $this->login, 'password' => $this->password, 'email' => $this->email,
'type' => Inscription::TYPE_MEMBRE, 'adresse_ip' => $_SERVER['REMOTE_ADDR']));
}
public function envoyerConfirmationMail()
{
$destinataire = $this->email;
$objet = "Inscription sur Lisla";
$emetteur = "";
$message = "Bienvenue sur Lisla.<br />"
."<br />"
."Vous pouvez dés maintenant jouer en vous connectant sur http://www.lisla.free.fr avec vos identifiants."
."Si vous avez la moindre question ou problème n'hésitez pas à lire la faq et à poster sur le forum dans le topic approprié."
."Je vous souhaite un bon jeu.<br />";
$message = wordwrap($message, 70, "\n");
mail($destinataire, $objet, $message,"From: $emetteur\r\nContent-Type: text/html;charset=\"iso-8859-1\"\r\n");
}
}
Enfin l'utilisation d'exceptions est-elle utile ici?
Merci si vous lisez tout ça.
RE: Améliorations possibles et conseils - Ter Rowan - 13-07-2010
pour les exceptions dès ce petit bout de code (je m'y suis mis après un volume bien plus grand et qu'est ce qu'on en ch.... à tout remettre) c'est quand même super confortable après
je verrais au moins à deux endroits ces exceptions :
dans chargerClasse : aucune raison d'avoir un message et puis c'est tout en cas de problème de fichier/classe
à la place du "or die" même principe faut tout exploser, certes mais avec "classe (ben oui on est en objet donc classe, ok je sors)
et sinon, même si je me pose encore la question pour moi même, au lieu d'avoir un $erreur qui se balade avec du texte dedans, je verrais bien une classe (pattern singleton) que tu appellerais pour stocker l'ensemble des messages et après l'envoyer à l'affichage (surtout, cela te permettrait d'annuler tout ce qui a été dit ou une partie, en cas d'exception levée par la suite)
RE: Améliorations possibles et conseils - zeppelin - 13-07-2010
Si tu veux mon avis, la première partie du code de inscription.php n'a rien à faire la...
A savoir:
Code PHP :
<?php
function chargerClasse($classname)
{
if(is_file('controleurs/'.$classname.'.class.php'))
require 'controleurs/'.$classname.'.class.php';
elseif(is_file('modeles/'.$classname.'.class.php'))
require 'modeles/'.$classname.'.class.php';
else
echo 'La classe demandée n\'existe pas.';
}
spl_autoload_register('chargerClasse');
session_start();
TOn site ne va pas comporter uniquement inscription.php, donc tu devra dupliquer ce fragment de code... et ça c'est NON direct! Il te faut un point de départ ailleurs, que ce soit index.php qui charge une class Core(); ou je ne sais quoi, à toi de voir.
Ensuite pourquoi fait tu ceci:
Code PHP :
<?php
include('class/bdd.class.php');
Je me permet un exemple, bien que très simple:
Code PHP :
<?php
function __autoload($className)
{
$paths[] = library . $className . '.php';
$paths[] = models . $className .'.php';
$paths[] = config . $className .'.php';
foreach($paths as $value)
{
if(file_exists($value))
{
include_once($value); // ou require_once, comme tu veux
break;
}
}
}
En tous les cas utilise _once, rien ne sert à la charger plusieurs fois!
Ensuite pense à mettre === pour checker les booléenes.
Code PHP :
<?php
if($erreur == FALSE)
// devient
if($erreur === FALSE)
// ou encore
if(!$erreur)
Ensuite pour ta classe, sans rentrer dans le détail, n'hésite pas à encore plus fractionner ton code en méthodes. Au lieu de ton if imposant, pourquoi ne pas déléguer tout ceci à des méthodes en privé? En cas de débuggage tu sera directement dans quel méthode chercher, en cas de changement la même chose...
Voilà en gros!
PS. Je plussoie vivement le singleton pour les messages d'erreur, comme conseillé en dessus! :-)
RE: Améliorations possibles et conseils - Shudrum - 13-07-2010
Pareil pour les erreurs.
De plus : même pour les mails : reste en MVC, ici tu n'es plus dedans :
Code PHP :
<?php
public function envoyerConfirmationMail()
{
$destinataire = $this->email;
$objet = "Inscription sur Lisla";
$emetteur = "";
$message = "Bienvenue sur Lisla.<br />"
."<br />"
."Vous pouvez dés maintenant jouer en vous connectant sur http://www.lisla.free.fr avec vos identifiants."
."Si vous avez la moindre question ou problème n'hésitez pas à lire la faq et à poster sur le forum dans le topic approprié."
."Je vous souhaite un bon jeu.<br />";
$message = wordwrap($message, 70, "\n");
mail($destinataire, $objet, $message,"From: $emetteur\r\nContent-Type: text/html;charset=\"iso-8859-1\"\r\n");
}
RE: Améliorations possibles et conseils - Ter Rowan - 13-07-2010
(13-07-2010, 07:23 PM)zeppelin a écrit : Ensuite pense à mettre === pour checker les booléenes.
Code PHP :<?php
if($erreur == FALSE)
// devient
if($erreur === FALSE)
// ou encore
if(!$erreur)
je pense que !$erreur correspond (en négatif) à == et pas ===
du coup if(!$erreur) donnera vrai si 0, false, null, ...
RE: Améliorations possibles et conseils - php_addict - 14-07-2010
c'est bien ce qu'il disait: il ne fait pas du MVC
(13-07-2010, 07:53 PM)Shudrum a écrit : De plus : même pour les mails : reste en MVC, ici tu n'es plus dedans :
d'ailleurs tu ferais comment Shudrum ???
RE: Améliorations possibles et conseils - Joojo - 14-07-2010
Merci à tous, j'ai quelques idées ainsi qu'un paquet de questions.
Je vais regarder tout ça, puis je créerai un double post.
RE: Améliorations possibles et conseils - wildd - 14-07-2010
pour la remarque de zepplin:
oublie le *_once.
__autoload n'est appelé que si php rencontre une erreur lors de l'instanciation d'une classe de type: j'ai pas la déf de cette classe => avant de lancer effectivement l'erreur il appelle __autoload pr voir si il arrive pas à régler le problème.
dans ces conditions tu risque difficilement de voir un fichier de déf de classe inclus plusieurs fois.
RE: Améliorations possibles et conseils - atra27 - 15-07-2010
Je me permet de préciser que require_once ainsi que include_once sont bien plus gourmand en ressources que les include/require normaux, donc a éviter quand c'est possible...
RE: Améliorations possibles et conseils - Joojo - 15-07-2010
Bon je vais y aller par étape. Tout d'abord le pattern Singleton, je le connaissais déjà mais que ce soit pour mes messages d'erreurs ou la connexion à la bdd je n'ai jamais comprit quelle était l'utilité ou la conséquence de ne pouvoir faire qu'une instance de la classe.
Pour mes messages d'erreurs je viens de créer ceci:
Code PHP :
<?php
class Test
{
private static $messages;
public function __construct($message)
{
self::$messages = self::$messages.'<br />'.$message;
}
public static function getMessages()
{
return self::$messages;
}
}
$test1 = new Test("Donuts");
echo Test::getMessages();
$test2 = new Test("Bière");
echo Test::getMessages();
Les booléens j'avais complètement zappé que cela existait. ^^' Merci cela m'a l'air beaucoup plus pratique pour vérifier avec des méthodes. Je vais plus fragmenter le code, avec l'aide de ceux-ci.
Mais en ce qui concerne les exceptions, je ne vois pas du tout comment les utiliser, ce qu'il faut en faire. Je suis un peu perdu sur leur bonne utilisation. Faut-il les utiliser pour afficher les messages d'erreurs comme "mot de passe incorrect" ou les possibles erreurs sql ou je ne sais quelles autre exceptions exceptionnellement exceptionnelles? ( ^^ suis pas très inspiré pour expliquer mon désarroi)
Donc ce que je comprend à propos des dires de Ter Rowan (attention aux yeux sensibles):
Code PHP :
<?php
try
{
(SELECT * FROM .......) or die(throw new Exception ('Erreur sql'));
}
catch (Exception $e)
{
echo $e->getMessage();
}
Je ne suis vraiment pas sur que cela fonctionne et l'utilité là est de 0. Donc je suis à côté de la plaque. En plus il faut encore utiliser une classe pour le traitement de ces exceptions..