JeuWeb - Crée ton jeu par navigateur
Critique de mon Premier Vrai Script de POO - Version imprimable

+- 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 : Critique de mon Premier Vrai Script de POO (/showthread.php?tid=2610)

Pages : 1 2 3


Critique de mon Premier Vrai Script de POO - Eluox - 27-05-2008

Bonjour, je me met a la POO, intensivement, et je voudrais votre avis sur ma première class.

Merci a Sephi pour les petits coups de main Wink
Ne pas faire gaffe au erreurs dans les commentaires en anglais, c'était fait vite fait Smile

Et pour info, MySQL c'est mysqli Smile
Code PHP :
<?php
/**
* Player
*
* @author El[u]ox <Lakers_suporter@hotmail.com>
* @copyright 2008
* @version 1.0
* @package Player
*/
class Player implements IPlayer
{
//Private Player Information
private $playerId;
private
$name;
private
$mail;
private
$password;
private
$avatar;
private
$signature;

public function
create($newPlayerInfo)
{
return
System::addPlayer($newPlayerInfo);
}

public function
load($playerID)
{
//Try to connect mysql
$mysql = Database::GetInstance();

//Select User Information
$sql = sprintf(
"SELECT email, name, password, avatar, signature
FROM player
WHERE id = %d;"
,
(int)
$playerID
);
//Do the Query
$aPlayer = $mysql->sqlFetch($sql,MYSQLI_ASSOC);

$this->playerId = $playerID;
$this->mail = $aPlayer[0]['email']; //And assigning user information
$this->name = $aPlayer[0]['name'];
$this->password = $aPlayer[0]['password'];
$this->avatar = $aPlayer[0]['avatar'];
$this->signature = $aPlayer[0]['signature'];
}

public function
getInfo($sKey) { //Recuperate an information
return $this->$sKey;
}

public function
changeAvatar($newURL) //Change Avatar
{

$Extension = strtolower(substr(strrchr($newURL,'.'),1));
$allowedExtension = array('jpg','bmp','gif','png');

if(
in_array($Extension,$allowedExtension) == false){
return
'Synthaxe URL incorrect';
exit;
}

$mysql = Database::GetInstance(); //Try to connect Mysql

$sql = sprintf("UPDATE player SET `avatar` = '%s' WHERE id = %d",strip_tags($newURL), $this->playerId); //Update query
$mysql->sqlQuery($sql); //Do the query

return 'Avatar Changé avec succès';
}

public function
changeSignature($newSign) //Change Signature
{
$Extension = strtolower(substr(strrchr($newSign,'.'),1)) ;
$allowedExtension = array('jpg','bmp','gif','png');

if(
in_array($Extension,$allowedExtension) == false){
return
'Synthaxe URL incorrect';
exit;
}

$mysql = Database::GetInstance(); //Try to connect Mysql

$sql = sprintf("UPDATE player SET `signature` = '%s' WHERE id = %d",strip_tags($newSign), $this->playerId); //Update query
$mysql->sqlQuery($sql); //Do the query

return 'Signature Changé avec succès';
}

/**
* @return string $this Dumping of $this
*/
public function toString() {
echo
'<pre>', var_dump($this), '</pre>';
}
}
?>

Merci a ceux qui m'aiideront.


RE: Critique de mon Premier Vrai Script de POO - Sephi-Chan - 27-05-2008

Salut,

Grosse remarque, pourquoi utiliser un tableau contenant les informations ? Rien ne le justifie du point de vue de la conception. Les attributs de ton objets (le nom, l'identifiant, l'email, etc.) doivent être des attributs.

Si c'est parce que tu as la flemme de faire des setName, getName, etc. tu peux implémenter les méthodes magiques __get() et __set(). Je te laisse te renseigner sur leur utilisation. Tu n'es pas obligé de détourner la nature d'un objet pour subvenir à tes besoins techniques. Le mieux reste cependant d'écrire tes setters et tes getters à la main.


Sephi-Chan


RE: Critique de mon Premier Vrai Script de POO - Ren Nelos - 27-05-2008

Bonjour,

Ce qui suit sont des remarques personnelles très influencées par des pratiques personnelles =]

Je commencerait par le début en disant, ça non :
Code PHP :
<?php 
//Private Player Information
private $player = array(
'name' => '',
'mail' => '',
'password' => '',
'avatar' => '',
'signature' => ''
);
Ca oui :
Code PHP :
<?php 
//Private Player Information
private $name;
private
$mail;
private
$password;
private
$avatar;
private
$signature;

D'accord, pas d'accord ? ^^


RE: Critique de mon Premier Vrai Script de POO - Eluox - 27-05-2008

Merci a vous.

J'ai modifier te éditer mon premier message.
Je suis toujours la pour plus de conseils.

Cordialement,


RE: Critique de mon Premier Vrai Script de POO - Ren Nelos - 27-05-2008

J'aurais une certaine tendance à dire qu'il faudrait un pattern singleton pour cette classe, non ?

Code PHP :
<?php 
class MaClasse {
private static
$instance;

private function
__construct() {
}

public static function
getInstance {
if ( !isset(
self::instance) ) {
self::$instance = new MaClasse();
}
return
self::$instance;
}
}

T'façon, tu dois savoir ce que c'est puisque tu l'utilises pour ta classe SQL, nan ? ;=]


RE: Critique de mon Premier Vrai Script de POO - naholyr - 27-05-2008

Je n'aurais pas mis la requête dans le constructeur, car sinon comment veux-tu créer un nouveau joueur, qui n'existe pas encore dans la base (par exemple à l'inscription) ?

J'aurais plutôt fait un vrai début de Data Object avec :
- les getters (très utile pour faire des traitements avant de renvoyer une valeur)
- les setters (très utile pour faire les vérification avant d'affecter la valeur)
- les loaders (va charger les infos depuis la bdd)
- le saver (sauve l'instance en cours dans la bdd)
ces deux derniers éléments imposant de stocker l'ID dans les attributs de l'objet Wink

Typiquement, l'utilisation donnerait ce genre de codes :
Code PHP :
<?php 
// Inscription
$player = new Player;
$player->setName($login);
$player->setMail($email);
$player->setPassword($password);
try {
$player->save();
} catch (
Exception $e) {
die(
'Impossible de créer le compte, erreur : ' . $e->getMessage());
}
ou
Code PHP :
<?php 
// Identification
$player = new Player;
if (
$player->auth($login, $password)) {
// il est loggé
$_SESSION['player_id'] = $player->getId();
}
ou encore
Code PHP :
<?php 
// Utilisateur est-il loggé ?
if (isset($_SESSION['player_id'])) { // il a l'air identifié
$player = new Player;
if (
$player->load($_SESSION['player_id'])) {
// oui, il est loggé
}
}

En résumé :
- Tous les getters et setters, en général ça ne sert pas à grand chose mais ça permet de gérer d'éventuels cas particuliers sans avoir à toucher au code plus tard. Par exemple tu peux faire des vérifications syntaxiques dans setEmail(), setPassword() ou setName() et lever une exception en cas de souci, ça permet de centraliser ce type de vérif Wink
- Stockage de l'ID de la bdd en attribut privé (s'il est null ça veut dire que l'objet vient d'être créé, sinon c'est qu'il a déjà été sauvé ou chargé depuis la bdd).
- Une méthode "load($id)" qui renvoie un booléen (true si l'id existe, false sinon) et en cas de succès charge toutes les infos dans l'objet depuis la bdd.
- Une méthode getId() mais pas de méthode setId() (ça n'aurait pas de sens de changer son ID, on utiliser load() pour ça).
- Une méthode "auth($login, $password)" qui marche comme load() mais qui se base sur le login et vérifie également le mot de passe.
- Une nouvelle méthode "save()" qui charge les infos du joueur en bdd. Cette méthode est assez simple : si l'attribut ID est défini, alors on fait un update et sinon on fait un insert. En cas d'erreur (login ou email déjà dans la base, ou autre erreur) on lève une exception dont le message est assez clair (destiné à être affiché).

Tout ça te donnera une classe bien complète et tu centraliseras vraiment toutes les actions propres à un compte utilisateur dedans.



Edit : pas d'accord pour le singleton, la classe Player ne représente pas forcément la session en cours, forcer un singleton poserait de gros soucis pour l'administration ou pour l'accès aux infos des autres joueurs (comment par exemple récupérer l'avatar d'un autre joueur que celui connecé ?).


RE: Critique de mon Premier Vrai Script de POO - Sephi-Chan - 27-05-2008

Je suis d'accord pour l'utilisation d'une méthode load(), je lui ai même précisément fait l'exemple de l'inscription ou la connexion d'un joueur. Smile


RE: Critique de mon Premier Vrai Script de POO - Eluox - 27-05-2008

Un grand merci a vous, je boss sur ça ce week end, et je vous montre le script Smile

Par contre, pour la création d'user, (setName, ect et save pour toi script), pourquoi ne pourrais-je pas faire create($name,$pass), ect


Cordialement, et merci


RE: Critique de mon Premier Vrai Script de POO - naholyr - 27-05-2008

Ce serait simplement moins homogène Wink Enfin c'est mon avis, ce genre de chose est assez subjectif.

De toute façon quand tu conçois une API il faut réfléchir en terme de besoins uniquement.
Quand tu écris une page web, tu pars du HTML que tu veux générer : tu as ton design, à partir duquel tu fais ta page HTML+CSS, à partir duquel tu écris ton code PHP qui se chargera de générer cet HTML. Tu pars du besoin, et tu y réponds.
Là c'est pareil : pars donc du code que tu aimerais avoir lorsque tu *utilises* ta classe Player, dans les différents cas d'utilisation, comme je l'ai fait plus haut. Ne pense pas à ce moment là à comment doit être faite la classe Player, juste à comment tu aimerais l'utiliser Wink et après tu réponds à ce besoin dans ta classe, tout simplement.

Tu verras que refléchir dans ce sens permet de simplifier la conception.


P.S: ou comment faire de la programmation pilotée par les tests sans le savoir Wink


RE: Critique de mon Premier Vrai Script de POO - Eluox - 27-05-2008

Merci beaucoup naholyr,
tu m'épatera toujours Smile