CLASSE : GESTION D'UTILISATEURS
codefalse
Messages postés
1123
Date d'inscription
mardi 8 janvier 2002
Statut
Modérateur
Dernière intervention
21 avril 2009
-
18 avril 2008 à 10:03
coucou747 Messages postés 12303 Date d'inscription mardi 10 février 2004 Statut Membre Dernière intervention 30 juillet 2012 - 19 avril 2008 à 13:21
coucou747 Messages postés 12303 Date d'inscription mardi 10 février 2004 Statut Membre Dernière intervention 30 juillet 2012 - 19 avril 2008 à 13:21
Cette discussion concerne un article du site. Pour la consulter dans son contexte d'origine, cliquez sur le lien ci-dessous.
https://codes-sources.commentcamarche.net/source/46396-classe-gestion-d-utilisateurs
https://codes-sources.commentcamarche.net/source/46396-classe-gestion-d-utilisateurs
19 avril 2008 à 13:21
19 avril 2008 à 13:07
J'ai pas regardé en détails, mais j'ai bien aimé dans l'ensemble. La plupart des remarques que j'avais à faire ont déjà été faites, donc je vais pas en rajouter une couche.
Juste deux petits trucs :
@Malalam : pour la page de codes, tu peux regarder en plein écran... On y voit mieux, c'est moins étriqué.
@Codefalse & Yoman : Pour se passer de la dernière ligne qui génère l'affichage, on peut utiliser la directive de configuration auto_append_file dans un fichier .htaccess
On peut ainsi avoir un petit fichier qui va gérer la fin de l'affichage, fermer ce qu'il y a à fermer, enregistrer ce qu'il y a à enregistrer, tout ça.
Mais comme Codefalse, je préfère garder le contrôle sur ce que j'affiche. Donc en général, j'ai dans chaque script, une ligne pour l'affichage de l'entête de ma page (jusqu'à la balise body et le premier div du contenu de ma page), et un pour le pied de page (en général qui comprend, outre la fermeture du premier div ouvert, le copyright, les liens vers la charte, mentions CNIL, tout ça, et la fermeture du document html). Plus quelques lignes pour l'affichage du document en lui-même.
J'ai donc 2 lignes qui reviennent toujours : parce que des fois, je n'affiche rien via mon template (pas de html : redirection, image à la volée, etc).
19 avril 2008 à 01:10
Mais perso je préfere faire un $obj->render ('tpl.phtml'); et ainsi controler l'avancement du rendu, plutot que de tout laisser faire le travail lui-même sans pouvoir agir en cas de soucis :)
18 avril 2008 à 20:17
Merci pour les commentaires, je ne connaissais pas session_set_save_handler, ça m'a l'air vachement intéressant et très pratique je vais voir comment j'pourrais adapter ça au code !
La classe core en fait était juste pour regrouper certaines choses et gêrer la construction (mais surtout la destruction) du moteur de template, donc elle est évidement plutot inutile.
Pour le getInstance de la classe Db c'est une bonne idée, ça va éviter des appels inutiles à la méthode sans arrêt :)
Les constantes dans le constructeur c'était juste parce qu'elle sont requise par la classe, elle ne sont pas requises si la classe est pas construite, mais c'est vrai que ça fait plus beau de les sortir.
Le __call je m'en sers pas dans les exemples (en fait si, une fois dans inscription.php: le code sur cette page), je voulais juste qu'il soit disponible parce que je trouvais l'idée intéressante :)
Comme j'ai dit le but de la source était surtout la classe users.class.php, le moteur de template était un test que je fesais, et du coup je me suis dit que ça serait intéressant de l'utiliser pour présenté un exemple de la source. Je vais peut être enlever le core.php suite aux remarques de Codefalse qui sont tout à fait judicieuse, il ne sert pas à grand chose si je rends le User singleton. Mais l'idée de faire une interface pour le template et éventullement le moteur de Db est tout à fait intéressante, ça va ajouter beaucoup à la flexibilité de la source !
Pour les exceptions je crois que tu as raison coucou747, je devrais ajouter une classe fille pour la gestion, mais je ne me sers pas des exceptions depuis très longtemps (enfin je les connais depuis bel lurette, mais c'est juste récement que j'ai vu leurs réel utilitée). Pour doxygen, je ne l'utilise pas. Moi je commente juste selon la syntaxe vu sur d'autres source de ce code lol, mais tu as raison je devrais aussi décrire les champs.
Ah ouais et le constructeur qui renvoit true ou false, c'était pour un test, j'ai oublier de l'enlever avant de poster pfff
Merci à tous, je vais bosser la dessus pour utiliser tous vos commentaires :) Décidément j'en apprends tous les jours ^^
PS: Codefalse: Si je ne génère plus le template avec le destructeur (je comprends ton point de vu, j'y avais même penser pendant un temps), ça veut dire que j'vais devoir ajouté une ligne pour faire le rendu à la fin de chacun de mes codes ? Ou ya p-e une méthode que je connais pas ?
18 avril 2008 à 19:40
bon à moi :-)
Je ne reviendrai pas sur le fait que ton bin's est intéressant.
Néanmoins, j'ai aussi quelques remarques et incompréhensions. Sur certains points, j'ai peut-être mal compris, ou pas tout vu (j'ai un peu de mal avec la nouvelle page permettant de voir les codes : c'est un peu petit à mon goût!).
Globalement, je trouve ta structure un peu bordélique. Ou je ne l'ai pas comprise, hein, c'est tout à fait possible.
Ta classe Core part d'une bonne idée, mais je la trouve sous exploitée. A mon sens, elle devrait être le véritable coeur de ton projet, et donc agencer le tout. Là, j'ai l'impression qu'elle fait la moitié du boulot.
La classe User : je ne comprends pas pourquoi tu fais appel au getInstance de ta classe DB pour chaque action. Tu passes par une méthode supplémentaire pour tout, alors que si tu l'instanciais dès le départ (vu que ta classe User a quasiment toujours besoin de cette classe dès qu'on l'instancie), tu gagnerais un peu. En sachant que les objets en php5 sont toujours des références, jamais des copies. Donc tu ne doublerais pas ton objet DB de cette manière.
Je ne comprends pas non plus la définition des constantes dans le constructeur. Pourquoi déclarer dans une classe des constantes globales ? Si tu en as besoin au global, sors les. Sinon, crée des constantes de classe.
Le __call() est rigolo, mais je ne vois pas où tu t'en sers. Tu passes directement par setValue(), ce qui me semble plus opportun en effet.
Tu aurais aussi ou te servir de l'extension filters, très intéressante.
J'ai aussi un peu de mal avec ton mIterator qui renvoie true ou false dans son constructeur ? Pourla méthode seek(), t'y es presque ;-)
Sinon c'est du bon travail! il y a des idées très intéressantes là-dedans.
18 avril 2008 à 14:13
Par ailleur je voulais ajouter un truc avant que j'ai omis :
Regarde du coté de Session_set_save_handler, peut-être connais-tu déjà ? En l'occurence pour ce genre de traitement, ca peut-être intéressant. (http://fr.php.net/manual/fr/function.session-set-save-handler.php)
Dans ma vision des choses (que je devrais faire prochainement vu que c'est dans la roadmap de mon framework :p), je verrai plus la gestion des utilisateurs dans un truc du genre
Une classe mere User (un peu comme la tienne dans les grandes lignes)
Une classe fille SessionUser, une autre classe fille DbUser, etc
Ces classes utiliseront session_set_save_handler pour définir là ou on enregistrera les données (de base, en fichier, en base de donnée, etc).
Il y aura aussi à coté une classe Authentication qui permet l'authentification de l'utilisateur, n'ayant besoin que du login et password et éventuellement des ACL...
Bonne continuation en tout cas :)
18 avril 2008 à 11:32
*tu commentes tes fonctions en doxygen, mais pas tes champs, c'est domage, parce-qu'en remplacant // par //!, tu peux ajouter de la doc.
*tu ne fais pas d'auto-loader de classes, ce qui te fait plein d'includes et multiplie les verifications comme : if (!class_exists ('User', true) ) {
*tu devrais mettre plus d'exceptions, la class Exception est normalement une base, on en fait des classes filles pour chaque type de cas, ca facilite l'utilisation du catch
18 avril 2008 à 10:03
Etant donné que j'envisage de faire un système utilisateur pour mon framework, ta classe m'intéresse :)
J'ai zappé les parties Db et Template, pour aller droit au but.
Que serait un commentaire sans des remarques ? :p
Déjà, au niveau core, tu impose le moteur de template (dans __construct : new SimpleTPL), et si je veux pas le tiens ? :p
Tu devrais plutot prendre en paramètre de __construct une classe ayant une interface spécifique que tu spécifie (genre iTemplate) (__construct (iTemplate $tpl);) comme ca tu sais quelle fonction elle utilisera puisque c'est toi qui les aura définies dans ton interface, et tu permettra une plus grande souplesse pour le moteur de rendu. Tu devrais d'ailleur faire peut-etre la meme chose pour la classe db ? A voir.
A mon avis (mais ca n'engage que moi), faire le rendu dans le destructeur, c'est dangeureux car si tu as une erreur, tu n'aura aucun rendu :/
Ensuite tu définit les regex de validation directement dans la méthode setValue. Personnellement je suis contre, car ca t'oblige à modifier une classe user pour ajouter un systeme de validation. Tu modifie un truc pour ajouter une chose qui n'est pas en rapport.
Tu devrais faire une classe indépendante (à mon avis) :p
J'aime bien l'idée d'utiliser __call pour ajouter des propriétés, vraiment pas con ! :=)
Personnellement au lieu de faire des isTaken...., j'aurai fait une fonction générique verify, qui retourne une constance LOGIN_TAKEN, EMAIL_TAKEN, etc. Mais ca c'est juste pour donner un avis, ta méthode fonctionne parfaitement quand meme ;)
A mon avis, ta classe core n'a rien à faire là. Concrètement, elle fait quoi ?
D'apres ce que j'en ai compris, elle permet la relation avec ton moteur de template et instancie la classe user. Pourquoi ne pas mettre le singleton user dans la classe user, et définir une instance de ton moteur de template directement ?
dans user :
public static getInstance () {
if (!isset (self::$_oInstance) || !(self::$_oInstance instanceof self))
self::$_oInstance = new User ();
return self::$_oInstance;
}
Et dans tes pages, tu fait :
$t = new simpleTPL ('./templates');
$u = User::getInstance ();
if ($u->isConnected ())
echo "ok :)";
else
$t->display ('login.phtml');
(grossomodo :p)
Bon d'accord !! j'arrete ! :)