Correction d'un code en poo

moijhd · 03-11-2009 19:09:39

Bonjour, je m'essaie à la programmation orientée objet en PHP !

J'ai écrit une classe que je voudrais que vous corrigiez !

[code php]
<?php
class Page{

// VARS

public $NomsAutorises = array();
public $Nom;

// CONST

const NomParDefaut = 'accueil';

// FUNC

// Constructeur
public function __construct($NomsAutorises = NULL, $Nom = NULL){
$this->AjouterNomsAutorises($NomsAutorises);
$this->AjouterNom($Nom);

}

// Ajoute un nom de page à autoriser
public function AjouterNomAutorise($NomAutorise){
if(!empty($NomAutorise)){
array_push($this->NomsAutorises, $NomAutorise);
}
}

// Ajoute un nom demandé
public function AjouterNom($Nom){
if(empty($Nom)){
$this->AjouterNomDepuisGet();
}
else{
$this->Nom = $Nom;
}

}

// Ajoute un nom demandé
public function AjouterNomDepuisGet(){
if(isset($_GET['page']) and !empty($_GET['page'])){
$this->Nom = $_GET['page'];
}
}

// Ajoute plusieurs pages à autoriser
public function AjouterNomsAutorises($NomsAutorises){
if(is_array($NomsAutorises)){
foreach($NomsAutorises as $NomAutorise){
$this->AjouterNomAutorise($NomAutorise);
}
}
else{
$this->AjouterNomAutorise($NomsAutorises);
}
}
// Ajoute le nom par defaut
public function AjouterParDefaut(){
$this->AjouterNom(Page::NomParDefaut);
}

// Verifie que le nom est vide
public function NomEstVide(){
if(empty($this->Nom)){
$Res = true;
}
else{
$Res = false;
}
return $Res;
}

// Vérifie que le nom de la page demandée est autorisé
public function EstAutorise(){
if(in_array($this->Nom, $this->NomsAutorises)){
$Res = true;
}
else{
$Res = false;
}
return $Res;
}

// Inclure
public function Inclure(){
// On regarde si le nom est vide
if($this->NomEstVide()){
$this->AjouterParDefaut();
}
// On regarde si la page est autorisée
if(!$this->EstAutorise()){
$this->AjouterParDefaut();
}
// On inclut
include($this->Nom.'.php');
}

}
$Page = new Page(NULL, 'page2');
$Page->AjouterNomAutorise('page1');
$Page->AjouterNomAutorise('page2');
$Page->AjouterNomsAutorises(array('page3', 'page4', '', 'page5'));
$Page->Inclure();
?>
[/code]

Ce code remplace mon ancien code (qui donne presque le même résultat) ;

[code php]
<?php
$Noms = array('page1', 'page2', 'page3');
if(isset($_GET['page']) and !empty($_GET['page']) and in_array($Noms, $_GET['page'])){
include($_GET['page'].'.php');
}
else{

} include('accueil.php');
?>
[/code]

Je me demande aussi quel est ici l'interet de l'objet...

Dernière modification par moijhd (08-11-2009 00:17:20)

xTG · 04-11-2009 07:19:12

Pas vraiment le temps pour le moment de regarder tout le code mais effectivement l'objet n'était pas très utile dans ce cas là.
Une simple fonction aurait fait l'affaire.

Maskime · 04-11-2009 13:32:21

Bonjour,

Pour commencer, que veux tu que nous corrigions ? Quelle est l'erreur qui t'es renvoyée ?

Ensuite pour ce qui est de la POO, tu as en fait réalisé ce qu'on appelle un contrôleur.
Maintenant il faut pas oublier que un des buts de la programmation orientée objet est de facilité l'évolution du code. Par exemple dans ton cas ca serait l'implémentation de l'url rewriting sans passer par un fichier .htaccess mais pas une BDD par exemple.
Au lieu de récupérer le tableau $_GET tu pourrais récupérer l'url complète et faire l'inclusion de la page demandée en te basant sur des règles que tu as défini dans une BDD. Ca permettrai d'optimiser le référencement de ton site sans tout péter dans ton code. Bien sûr pour réaliser cette archi il te faut un autre objet dont le travail sera d'interpréter les urls et de les générer en fonction des types de liens...
Tu pourrais aussi rajouter une gestion de droit facilement. Ce serait un autre objet qui travaillerait avec des objets qui représenteraient tes urls...

Autre but de la POO : La réutilisation du code.
Ta classe d'inclusion tu pourrais aussi bien la mettre sur un autre site, ça marcherait aussi.

Concrètement :
Est ce que c'était franchement utile de le faire comme ça ? -> Non
Est ce que c'était bien de le faire comme ça ? -> OUI !!!!

Il faut quitter cette habitude de coder en procédurale... L'objet c'est très très très très bien !!!

moijhd · 04-11-2009 17:51:55

Ok !

Maintenant, sur le contenu :

Aurait-il pu être "mieux" de creer une class PagesAutorisees, PageDemande, Inclusion (la première version que j'ai fait avait cette forme)...enfin de séparer en plusieurs classes courtes ?
Les fonctions sont-elles logiques ?

Plus particulièrement : toute critique est attendue ^^

Maskime · 04-11-2009 18:56:26

Alors,

Quand on fait de l'objet tu as 2 types de politiques :
Politique stricte (utilisé dans les langages à typage fort genre Java et C# .Net)
Politique souple (utilisé uniquement avec les langages qui le permettent genre PHP et JS)
[Ces termes n'ont rien conventionnel, c'est du cru à moi perso ]

Quand tu es dans une politique souple il faut savoir s'arrêter dans l'"objetisation"...
Ca veut dire qu'il faut savoir faire de l'objet utile, si tu commence à faire des objets pour tout et n'importe quoi, tu vas rendre ton code lourd à la lecture et donc rendre la maintenance plus dure pour quelqu'un qui n'a jamais touché à ton code...
Si tu commence à faire un objet dans lequel tu te rends compte qu'il n'y a en fait que 1 ou 2 membres... C'est peut être pas franchement utile, un tableau peut faire l'affaire. En revanche si tu code une fonction qui doit être capable de renvoyer plusieurs valeurs :
ex : Imagine que dans ton objet Page tu ai une méthode info() qui te donne le nom de la page, chemin sur le disque, la date de création et l'utilisateur a créé la page
Dans un cas comme celui-ci, le principe "D'encapsulation" de la programmation objet doit t'obliger à créer un objet de type PageInfos avec des getters et des setters pour que tu puisse récupérer le contenu de ton objet sans savoir comment est construit l'objet.

Voilà pour la réponse sur la liste d'objet

Pour ce qui est de tes méthodes, elles me semblent assez logique et intelligentes.

Si tu veux faire le truc en un peu plus pro :
1. Tu commences par coder en anglais (ça parait rien mais ça évite les phrases de "franglais" dans le code qui n'est compréhensible que par l'auteur du code...)
2. Utilise une notation à dos de chameaux en mode Java (on commence par des minuscules)
3. Préfixe tes variables par le type attendu : $sPageName veut dire que l'on attends un "string" dans cette variable $oPageName veut dire que l'on attends un "objet" etc
4. Utilise un préfixage différent pour :
Tes membres de classe : $m_aNomsAutorises
Tes paramètres de méthodes : $p_aNomsAutorises
5. Nomme tes constantes en MAJUSCULES : NOM_PAR_DEFAUT
6. Evite d'utiliser des chaines de caractères sorties de nulle part dans ta classe : $_GET['page'] => $_GET[self::PAGE_URL_PARAM] (où PAGE_URL_PARAM est un constante de ta classe)
7. Utilise la javadoc pour le commentaire de tes classe :
/**
* @desc Add a name to the authorized page list
* @param string $NomAutorise Page name to add
* @return void
*/
public function AjouterNomAutorise($NomAutorise)

8. Il faut faire 1 fichier par classe (et c'est tout!!!!)
9. Ordonner les classes dans des dossiers intelligents !
10. Les noms de classe commencent avec des majuscules
11. Les classes sont stockées dans des fichiers nommés comme ça : MaClasse.class.php

Voilà respecte ça déjà et tu feras du code bien propre et maintenable.
Si tu veux en savoir plus sur le sujet, essaye de faire un tour sur developpez.com, tu y trouveras des tutos bien fourni

Bonne soirée

Dernière modification par Maskime (05-11-2009 14:15:44)

moijhd · 04-11-2009 19:19:48

Ok merci !

Je vais revenir sur quelques points :

2. Quelle est cette notation à dos de chameaux ?

3. De façon générale, quel "forme" de noms devrais-je donner à mes variables ? ($mavariable, $MaVariable, $Ma_Variable, $ma_variable, ($my_variable), ...). De meme, que faire pour les noms de fonctions ?

Par ailleurs, j'avais essayé le plus possible de faire des fonctions sans argument : cela a-t-il un interet ?

Et si je sépare ma class en deux, à savoir une class PageDemandee et une class PagesAutorisees, où devrait avoir lieu le test "savoir si la page est autorisé ? dans la class PageDemandee où je construit une fonction avec argument PagesAutorisees ? Dans la class PagesAutorisees ? Dans une autre class ? Ou alors dans PageDemandee crée une nouvelle variable $autorisee ?

Merci !

PS : je vais modifier mon code est proposé autre chose d'équivalent

Dernière modification par moijhd (04-11-2009 19:37:51)

Maskime · 05-11-2009 12:21:38

Hello,

2 -> Alors notation à dos de chameaux, consiste à alterner majuscule et minuscule dans un nom de variable :
$sCeciEstNoteEnDosDeChameaux
3 -> je te renvoi au point 2 et 1 => $sMyVariable (anglais, dos de chameaux et préfixage)

moijhd a écrit :

Par ailleurs, j'avais essayé le plus possible de faire des fonctions sans argument : cela a-t-il un interet ?

Il n'y a pas de règle concernant ce point il faut que tu fasse en fonction du besoin.

moijhd a écrit :

Et si je sépare ma class en deux, à savoir une class PageDemandee et une class PagesAutorisees, où devrait avoir lieu le test "savoir si la page est autorisé ? dans la class PageDemandee où je construit une fonction avec argument PagesAutorisees ? Dans la class PagesAutorisees ? Dans une autre class ? Ou alors dans PageDemandee crée une nouvelle variable $autorisee ?

Tu as un problème de conception ici. Tes classes PageDemandee et PageAutorisee sont en fait des objets Page le fait que la page soit autorisée ou pas ne les concernent pas.
Donc la bonne méthode si tu veux faire comme ça c'est d'avoir 2 objets :
Page (Page.class.php)
PageControler (PageControler.class.php)
(je te rajoute un point '10' à ma liste des bonnes pratiques => Les noms de classe commencent avec des majuscules et un point '11' => Les classes sont stockées dans des fichiers nommés comme ça : MaClasse.class.php)
L'objet PageControler doit comporter une méthode qui à partir de l'url et des paramètres passés dans celle-ci, te construit un objet de type Page. Une autre méthode de cet objet PageControler reçoit en param l'objet Page et cette méthode te retourne "true" ou "false" pour te dire si elle autorisées ou pas.

Je te laisse essayer de pondre un code à partir de ça et on corrigera après

Maljuna Kris · 06-11-2009 13:25:35

Saluton,
Si je souscris bien volontiers à la plupart des recommandations de Maskime, j'émets une grosse réserve :
- sur la recommandation n°1

Tu commences par coder en anglais

en tant que militant espérantiste contre l'hégémonie de l'anglais, je ne pouvais laisser passer cela.

Pour le reste, c'est affaire, comme souvent, de bon sens. La POO, c'est super sympa, mais ce n'est pas la panacée à mettre à toutes les sauces, pour tout, et n'importe quoi.
Je vais me citer, je dis souvent dans mes interventions de formateur la chose suivante :

De nous jours l'informatique se mêle d'à peu près tout parce qu'elle permet de faire n'importe quoi.
Or, c'est justement ce qu'il faut éviter.

Maskime · 06-11-2009 14:13:53

Je retiens le dicton ^^ et je mettrai un copyright à chaque utilisation

moijhd · 06-11-2009 20:44:08

Nouvelle version :

[code php]

<?php
class Page{

// VARS

public $Name = NULL;

// CONST

const PAGE_URL_PARAM = 'page';
const DEFAULT_NAME = 'accueil';

// FUNC

// Construct
public function __construct($Name = NULL){
$this->NewName($Name);
}

// Ajoute le nom
public function NewName($Name){
// on regarde si le nom est vide
if(empty($Name)){
// Le nom est vide
// On regarde si un nom est passé en argument
$this->NewNameFromUrl();
}
else{
// On donne un nom à la page
$this->Name = $Name;
}
}

// Ajoute le nom à la page par récupération de l'url
public function NewNameFromUrl(){
// On regarde si un nom de page est passé dans l'url
if(isset($_GET[self::PAGE_URL_PARAM]) and !empty($_GET[self::PAGE_URL_PARAM])){
// On donne le nom dans l'url à cette page
$this->NewName($_GET[self::PAGE_URL_PARAM]);
}
else{
// On ne fait rien
$this->NewDefaultName();
}
}

// Donne le nom par defaut à la page
public function NewDefaultName(){
$this->Name = self::DEFAULT_NAME;
}
}
?>

<?php
class PageControler{

// VARS

public $AuthorisedPageNames = array();

// CONST

// FUNC

// Constructeur
public function __construct($PagesToBeAuthorised = NULL, $PageToBeIncluded = NULL){
// On récupère le tableau de pages ou la page à autoriser
$this->NewAuthorisedPages($PagesToBeAuthorised);
// On regarde si la page à inclure est effectivement une page
if($this->IsPage($PageToBeIncluded)){
// C'est une page
// On tente de l'inclure
$this->IncludePage($PageToBeIncluded);
}
else{
// Ce n'est pas une page
// On tente d'inclure une page vide URL
$this->IncludePage(new Page());
}
}

// Ajoute un nouvelle page à autoriser
public function NewAuthorisedPage($PageToBeAuthorised){
// On regarde si la nouvelle page à autoriser est effectivement une page
if($this->IsPage($PageToBeAuthorised)){
// C'est une page
// On l'ajoute
$this->NewAuthorisedPageName($PageToBeAuthorised->Name);
}
}

// Ajoute un tableau de nouvelles pages à autoriser
public function NewAuthorisedPages($PagesToBeAuthorised){
// on regarde si une page ou si un tableau de pages est passé en argument
if(!empty($PagesToBeAuthorised)){
// On regarde si c'est un tableau de page
if(is_array($PagesToBeAuthorised)){
// Pour chaque page du tableau
foreach($PagesToBeAuthorised as $PageToBeAuthorised){
// On tente d'autoriser la page
$this->NewAuthorisedPage($PageToBeAuthorised);
}
}
else{
// Pour l'unique page
// On tente de l'autoriser
$this->NewAuthorisedPage($PagesToBeAuthorised);
}
}
}

// Ajoute un nouveau nom de page à autoriser
public function NewAuthorisedPageName($PageNameToBeAuthorised){
// Si le nom n'est pas vide
if(!empty($PageNameToBeAuthorised)){
// On ajoute le nom à la liste
array_push($this->AuthorisedPageNames, $PageNameToBeAuthorised);
}
}

// Vérifie qu'un nom est autorisé
public function IsPageNameAuthorised($PageName){
return in_array($PageName, $this->AuthorisedPageNames);
}

// Vérifie qu'un nom est autorisé
public function IsPageAuthorised($Page){
return $this->IsPageNameAuthorised($Page->Name);
}

// Inclure une page
public function IncludePage($PageToBeIncluded){
// On vérifie que $PageToInclude est bien un objet de type page
if($this->IsPage($PageToBeIncluded)){
// On tente d'inclure la page par son nom
$this->IncludePageFromPageName($PageToBeIncluded->Name);
}
else{
// On crée une nouvelle page à inclure
$this->IncludePage(new Page);
}
}

// Inclure une page à partir de son nom
public function IncludePageFromPageName($PageToBeIncludedName){
// On vérifie que le nom de la page est autorisé
if($this->IsPageNameAuthorised($PageToBeIncludedName)){
// On ne fait rien
// Le nom du fichier
$FileName = $PageToBeIncludedName.'.php';
}
else{
// On appelle la page par defaut
// Le nom du fichier
$FileName = Page::DEFAULT_NAME.'.php';
}
// On inclut
include($FileName);
}

// Verifie que des noms sont autorisés
public function IsEmptyAuthorisedPageNames(){
return empty($this->AuthorisedPageNames);
}

// Vérifie qu'une variable est un objet page
public function IsPage($Object){
return is_a($Object, 'Page');
}
}
?>

<?php
class PageToBeIncluded extends Page{

// VARS

public $Title = NULL;

// CONST

const DEFAULT_TITLE = 'Titre par défaut';

// FUNC

// Constructeur
public function __construct($Name = NULL, $Title = NULL){
$this->NewName($Name);
$this->NewTitle($Title);
}

// Donne un titre à la page à afficher
public function NewTitle($Title){
// On regarde si le titre est vide
if(empty($Title)){
// Le titre est vide
// On donne le titre par defaut
$this->NewDefaultTitle();
}
else{
// On donne le titre
$this->Title = $Title;
}
}

// Donne le titre par défaut à la page
public function NewDefaultTitle(){
$this->NewTitle(self::DEFAULT_TITLE);
}

}
?>

[/code]

On critiquera le tout

Dernière modification par moijhd (07-11-2009 01:12:36)

Pierrot · 08-11-2009 15:52:27

on peu critiquer ??

pourquoi faire simple quand on peu faire compliqué :D
a++

moijhd · 08-11-2009 17:04:14

Ce n'est pas très constructif !

Considérons que le but est de "faire de la poo', que ce que l'on fait n'est qu'un prétexte et que l'on peut essayer de le faire bien quand même.

Pierrot · 08-11-2009 17:22:27

faire de la POO pour faire de la POO
si c'etait ton but, il est atteint
a++

moijhd · 08-11-2009 18:23:48

Donc il n'y a rien à redire sur tout ce qu'il y a écrit ? Même pas un petit détail ? Même pas une amélioration à apporter (et pas à ajouter) ?

Pierrot · 08-11-2009 18:29:13

y fait quoi ton objet ?
j'ai rien compris moi
a vrai dire, j'ai pas suivi
0 pointé :D
a++

Maljuna Kris · 09-11-2009 08:39:06

Saluton,
A ta décharge, Pierrot, POO ou pas, mais encore plus en POO, les commentaires explicitant les finalités d'une classe constituent un élément déterminant de son utilisabilité.
Une classe est censée être, a minima, une boîte noire avec laquelle on peut dialoguer via ses méthodes et attributs publics.
Cette "API" doit faire l'objet d'un commentaire exhaustif en début de déclaration de classe.
Et je ne parle même pas des possibilités d'extension de la classe.

Maskime · 10-11-2009 21:13:40

Hello,

Ce que je reproche à ton code :
1. La méthode IsPage() n'est pas franchement utile, mais c'est une bonne idée Depuis PHP5 tu peux forcer les types d'objets attendu dans la fonction en précisant leur type dans la déclaration :

public function IncludePage(Page $PageToBeIncluded){[...]}

 

2. Même si on fait de l'objet pour faire de l'objet, je ne vois pas trop à quoi sert ton objet PageToBeIncluded ?
3. Et plus important, tu n'as pas posté ton 'main' le script 'central' qui va exécuter ton code. Parce que c'est bien beau de faire des objets, mais si tu t'en sers pas c'est pas vraiment la peine Je te dis ça surtout parce que tu peux pas faire de conception parfaite du premier coup. Alors tu fais un premier truc, tu t'en sers et s'est quand tu t'en sers que tu te rends compte qu'il y a des choses qui ne sont pas pratiques, des choses que tu peux améliorer...

Donc fais ton main et poste le et à partir de là on pourra commencer à parler comment tes objets s'articulent entre eux

Bon courage

moijhd · 11-11-2009 12:38:26

2. PageToBeIncluded sert juste à dire que j'ai d'une part les pages autorisées et d'autre part, la page que je veux afficher, et donc, pour la page à afficher, je vais pouvoir donner plus d'informations à la page à savoir, pour l'exemple, le titre.

3. J'ai bien evidement fait des tests, je publierais donc un "usage type" ou du moins, la façon dont je désire l'utiliser !

Enfin, je me pose le problème suivant :
Il ne me semble pas très logique de vouloir pour chaque nouveau changement de page charger, c'est-à-dire ré-éxécuter, la partie où "j'ajouterais les pages autorisées". Je pense donc, à la "première connexion", chargée les pages, puis mette mon objet dans une session pour le réutiliser dans les pages suivants ?

Je développerais cela dans mon prochain message

PHP|Débutant :: Forums

Advertisement

#1 03-11-2009 19:09:39

Correction d'un code en poo

#2 04-11-2009 07:19:12

Re : Correction d'un code en poo

#3 04-11-2009 13:32:21

Re : Correction d'un code en poo

#4 04-11-2009 17:51:55

Re : Correction d'un code en poo

#5 04-11-2009 18:56:26

Re : Correction d'un code en poo

#6 04-11-2009 19:19:48

Re : Correction d'un code en poo

#7 05-11-2009 12:21:38

Re : Correction d'un code en poo

#8 06-11-2009 13:25:35

Re : Correction d'un code en poo

#9 06-11-2009 14:13:53

Re : Correction d'un code en poo

#10 06-11-2009 20:44:08

Re : Correction d'un code en poo

#11 08-11-2009 15:52:27

Re : Correction d'un code en poo

#12 08-11-2009 17:04:14

Re : Correction d'un code en poo

#13 08-11-2009 17:22:27

Re : Correction d'un code en poo

#14 08-11-2009 18:23:48

Re : Correction d'un code en poo

#15 08-11-2009 18:29:13

Re : Correction d'un code en poo

#16 09-11-2009 08:39:06

Re : Correction d'un code en poo

#17 10-11-2009 21:13:40

Re : Correction d'un code en poo

#18 11-11-2009 12:38:26

Re : Correction d'un code en poo

Pied de page des forums