Il aurait été judicieux de namespacer tes classes car une classe News ou Movie c'est trop générique. Si je développe une application qui contient déjà une classe News, il y aura conflit. Tu as trois possibilités pour éviter ça :

  * Tu préfixes tes classes par AlloCine par exemple (AlloCineMovie, AlloCineNews...)
  * Tu utilises la nomenclature PEAR / Zend et tu fournis l'autoloader qui va bien (AlloCine/News.php == classe AlloCine_News)
  * Tu utilises PHP 5.3 et les namespaces :

<?php

namespace AlloCine;

class News {

}

?>

Et quand tu l'instancies tu fais :

<?php

$news = new AlloCine\News();

?>

Ou alors:

<?php

use AlloCine\News;

$news = new News();

?>

Comme ça, si j'ai deux classes News dans mon projet, je peux les utiliser sans conflit de cette manière:

<?php

use MyApp\News as News;
use AlloCine\News as AlloCineNews;

$news = new News();
$allocine = new AlloCineNews();

?>

Hugo.

Hello,

Ta source est plutôt bien faite, bravo. Au niveau améliorations et simplification tu peux retirer le __construct() ne contenant qu'un appel au constructeur parent. Par défaut, PHP appel le constructeur parent donc on peut se passer de réimplémenter le constructeur si c'est uniquement pour appeler celui du parent. D'autre part, tu devrais retirer les commentaires de séparation de tes méthodes. Ils ne servent à rien si ce n'est d'alourdir la lecture du code.

Je te conseille plutôt d'avoir recours au mot-clé protected plutôt que private pour faciliter les héritages de tes classes. Dans tes constructeurs, il serait judicieux de passer au moins paramètre (obligatoire ou facultatifà : le nombre d'éléments par page:

public function __construct($nbByPage = 10)
{
  $this->nbByPage = (int) $nbByPage;
}

Enfin pour le pager SQL, ton objet PDO je ne devrait pas être instancié directement dans ta méthode mais plutôt passé par dépendance au constructeur par exemple. Ca te permettra deux choses :

* Pouvoir réutiliser un objet PDO existant
* Faciliter les tests unitaires de tes classes en "mockant (simulant)" l'objet PDO avec un objet qui se comporte comme lui sans faire les requêtes SQL sur la base.

Ce qui donne :

class PagerSql extends Pager
{
    protected $pdo, $table;
    
    public function __construct(PDO $pdo, $table = null, $nbByPage = 10)
    {
        parent::__construct($nbByPage);
        
        $this->pdo = $pdo;
        $this->table = $table;
    }
}

Je te laisse améliorer ton code en conséquence ;)

Hugo.

@Amteur : ah bon tu penses que c'est légal de voler le contenu d'un site Internet sans en avoir les autorisations ? Cool dans ce cas, je vais pomper le contenu du tien pour faire un site :)

Je vais être un peu critique bien que globalement le code est pas mal mais il y'a quelques petites choses qui me titillent.

1/ Concevoir ton calendrier dans une classe aurait été beaucoup plus pertinent car actuellement ton code n'est pas véritablement réutilisable... L'idéal serait de faire une classe Calendar et de l'appeler de cette manière :

$calendar = new Calendar($_GET['m'], $_GET['y']);

echo $calendar->render(); // Génère le tableau

// ou

echo $calendar; // si ta classe implémente la méthode __toString()

2/ Pour gérer les dates, préfère les fonctions natives de PHP comme strtotime... Tu t'embêtes à tester quel jour, quel mois et quel année on est pour pouvoir en déduire les suivants et précédents. Pour avoir le mois suivant, je fais simplement :

$timestamp = mktime(0,0,0,date('m'), 1, date('Y')); // premier jour du mois courant
$suivant = date('m', strtotime('+1 month', $timestamp));    // numéro du mois suivant

Encapsule tout ça dans une classe en sortant les $_GET à l'extérieur de la classe bien sûr. Au final ce sera beaucoup plus réutilisable que maintenant mais aussi testable unitairement avec PHPUnit ou Lime ;)

++

@CodeFalse : la source en elle même. Elle permet de réaliser une opération illégale.

Ce n'est pas très légal tout ça...

Je n'ai pas envie de partir dans un troll mais je vais revenir sur quelques points pour clôturer mes remarques. Je n'ai rien contre le fait qu'il partage ses sources mais le problème de Codes Sources c'est que la plupart des codes déposés ici sont très mauvais. Sur 10 codes déposés, il doit à peine y'en avoir un qui tient la route. Donc, à force de voir des codes médiocres, on ne peut s'empêcher de critiquer et parfois de descendre les sources (et leurs auteurs) pour leur montrer la mauvaise qualité de leur code. Je ne critique pas pour le plaisir de critiquer. J'essaie un tant soit peu d'être constructif et de justifier mes remarques, parfois pas toujours...

Quant à symfony (note que ça s'écrit avec un F et non PH !!!), je rigole de ta remarque pas véritablement justifiée. La chasse aux millisecondes ça ne sert strictement à rien, sauf si tu t'appelles Yahoo, Google ou Amazon. Et encore, ces derniers sites ont d'excellents temps de réponse car ils ont d'une part l'argent pour se payer les serveurs qui vont bien mais surtout car les optimisations sont réalisées côté client (optimisation du temps de chargement des pages côté navigateur). Donc on se moque de gagner 5 ou 10 ms sur un serveur lorsque l'on travaille avec un framework orienté objet. Le but d'un framework OO comme symfony ou Zend Framework, c'est d'apporter une structure cohérente et solide pour simplifier, industrialiser et professionnaliser les développements dans le but de les rendre plus pérennes et plus maintenables. D'autant plus que lorsque tu utilises ce genre d'outils, tu as généralement le serveur qui va bien derrière avec au minimum un cache d'opcodes. Si tu veux des ultra hautes performances, alors dans ce cas, tu commences par installer ton serveur web (Apache ou LightHTTPD par exemple) configuré uniquement pour tes besoins (cad que tu lui dégages toutes les lib dont ton projet n'a pas besoin), tu fais de même pour PHP. Tu codes ensuite entièrement en procédural, tu mets du cache de partout, tu optimises côté client (réduction des js et css, optimisation du html, compression gzip...), tu fais de jolis indexes dans ta BDD, tu crées des vues. Ok tu seras plus performant (ou pas !) mais ton projet deviendra très certainement laborieux à faire maintenir et à pérenniser. Sans parler du budget conséquent que ça nécessite !!! C'est bien pour cette raison que Google, Yahoo ou Amazon peuvent se le permettre. A titre d'information, Amazon perd 1% de chiffre d'affaire par jour si les temps de réponse de ses pages augmentent de quelques pouillèmes de millisecondes. Mais mis à part eux, je pense pas qu'il soit véritablement nécessaire de se casser le cul pour 5 ms, sauf si le client est prêt à sortir quelques gros billets. Mais ça, ça se négocie. L'approche pragmatique est bien souvent la meilleure ;)

++

PS : symfony 1.3 et symfony 2.0 seront bien plus performants que les versions actuelles. Mais sache que ZF, Jelix, CakePHP ou n'importe quel autre framework ne sont guère plus performants.

@Akhénathon : je ne dis pas que l'exemple que je donne est celui qu'il faut suivre, je donne ni plus ni moins qu'une piste de réflexion. Et pour ta gouverne, je sais très bien ce qu'est une CSRF.

Concernant l'indentation et l'anglais, ça a beau être du chipottage pour toi, mais ça ne l'est pas pour moi pour des raisons évidentes. Savoir indenter son code et respecter des conventions de codage reconnues est selon moi la première chose à avoir en tête lorsque l'on développe. En effet, on développe rarement que pour soi-même. Quand on développe en milieu professionnel dans une équipe ou bien lorsque l'on décide de mettre sa source à disposition du public, c'est la moindre des choses de rendre cette dernière lisible et compréhensible. Et bien sûr, cela passe par l'indentation, le choix des noms des méthodes et des classes mais aussi des commentaires (PHPDoc si possible). Quand on lit une source bâtie sur ces quelques règles élémentaires, ça donne déjà une bonne impression sur le travail du développeur et sur son niveau de compétence. Je travaille chez Sensio, la société qui édite le framework professionel open-source symfony. Clairement, on ne verra jamais passer ce genre de code dans nos projets !

Concernant les namespaces, je ne vois pas en quoi ce n'est pas optimisé. Je serai curieux de connaître l'argument qui te faire dire ça... Un tableau à 2 dimensions n'est pas moins performant qu'un tableau à une dimension. Bien que PHP permette de stocker des objets en session, ce n'est pas du tout une bonne pratique pour des raisons de performances et de place. Il vaut mieux s'arranger pour stocker une représentation de l'objet sous forme d'un tableau sérialisé.

Ensuite, ta remarque sur les performances du calcul de la salt et d'un jeton anti CSRF est totalement invalide. Calculer un hash et une valeur aléatoire, c'est peanut comme temps de traitement. Il faut arrêter avec la performance. Quand on veut travailler avec des objets et de la sécurité, on sait pertinemment que ça va coûter quelques pouillièmes de micro secondes supplémentaires de temps CPU. Je travaille quotidiennement avec symfony, framework entièrement OO et intégrant toute une panoplie de traitement pour sécuriser l'application développée contre les XSS et CSRF. Oui, clairement les performances ne sont pas les mêmes qu'avec une appli procédurale. Mais c'est normal, on fait de l'objet et de la sécurité. Et au final, les serveurs tiennent très bien la charge et délivrent leurs pages très bien. Il n'y a qu'à voir Dailymotion ou Yahoo pour s'en convaincre qui utilise symfony. Donc la notion de performance ça ne veut rien dire en développement web. Et sachant qu'acheter un serveur coûtera toujours moins cher qu'embaucher un développeur pour optimiser l'application, on se moque de gagner 5 ms dans un programme PHP.

Je reviens sur ta remarque concernant le hacker et la viabilité du jeton de protection. Certes les fichiers de session sont présents sur le serveur. Et ? En quoi le hacker pourra-t-il les lire ? La seule manière pour lui d'avoir accès à ces fichiers est de s'introduire physiquement sur la machine ou bien d'y introduire un script malveillant (par le biais un d'un upload non sécurisé par exemple). Sauf que s'il arrive à s'introduire sur le serveur, c'est que la sécurité de l'application est vraiment merdique ! Pour ta gouverne, configurer le répertoire de stockage des sessions fait également partie de la sécurité des sessions. Ca commence par reconfigurer manuellement le répertoire de stockage des sessions qui est /tmp par défaut. Dans le cas d'un serveur mutualisé, on prend aussi garde à ne pas écrire les fichiers de session de tous les sites hébergés dans le même répertoire. Chaque site doit avoir son dossier de sessions, hermétiquement clos et inaccessible par les autres.

Son système de session est très loin d'être performant et d'être évolutif. En effet, il ne permet que de "gérer" (c'est un bien grand mot) les sessions natives de PHP. Mais si j'ai envie de gérer mes sessions dans une base de données par exemple sans avoir à toucher l'implémentation de mon code métier. Comment je fais avec son code ? Bah on ne fait rien, on est grillé. On ne peut pas changer ou faire évoluer efficacement le gestionnaire de sessions.

Bref, ma note n'est pas injustifiée et je la maintiens. Cette classe est clairement "inutilisable" dans un environnement professionnel.

++

Hugo.

Et je note 4/10 au passage.

Dans mon exemple $salt c'est $_SESSION['salt'] ;)