CLASS LANG AVEC COOKIE + HTTP_ACCEPT_LANGUAGE

Commentaire de neigedhiver le 16/12/2009 02:23:23

Salut,

Je viens de survoler ton code et à première vue : ça me plaît énormément. C'est propre, clair et, cerise sur le gâteau, tu utilises correctement readdir() (c'est peut-être un détail pour vous, mais pour moi ça veut dire beaucoup lol plus sérieusement, c'est un peu un de mes chevaux de bataille...)

Quelques remarques :

else if (avec un espace) est un peu moins performant que elseif (sans espace). La raison : PHP interprète le else, puis le if et crée alors une seconde boucle if. Alors que sans espace, PHP sait qu'il s'agit de la même boucle. Oui, c'est vraiment du pinaillage (mais bon, comme mes remarques ne concernent que des détails, je les relève quand même ^^)

La lecture du tableau de langue me paraît... comment dire... trop compliquée pour ce que c'est. Visiblement, tu utilises une syntaxe type fichier INI. Pourquoi ne pas utiliser la fonction native de PHP parse_ini_file() ? C'est quand même plus simple et... BEAUCOUP plus performant, puisque natif à PHP (donc codé en C). Par ailleurs, comme tu utilises une PCRE pour parser chaque ligne, c'est très gourmand en ressources.

Pourquoi ne pas utiliser un itérateur pour parcourir le répertoire, plutôt que readdir() ? Même si tu l'utilises correctement, contrairement à 95% des auteurs des codes qu'on trouve sur phpcs, c'est quand même plus "pratique", plus lisible, plus facile à maintenir et peut-être même plus performant (mais je n'ai aucune conviction pour ce dernier point). Ça donnerait un truc du genre :

private function GetLangs() {
  foreach ($it = new DirectoryIterator($this->subdir . $this->path) as $file) {
    if ($file -> isFile() && '.lng' == substr($file->getFilename(), -4)) {
      $this -> listLangs[] = $file -> getFilename();
    }
  }
}

Ici, l'utilisation de substr() est plus rapide qu'une PCRE, qui n'est par ailleurs pas nécessaire pour un test aussi simple.

La syntaxe :
var $lang;
est une syntaxe PHP4. Même si ça fonctionne encore en PHP5 (pour des raisons de compatibilité) et que c'est rigoureusement à la syntaxe PHP5 (par défaut, une propriété dont la visibilité n'est pas définie, comme c'est le cas avec var, celle-ci est publique), il convient d'écrire :
public $lang;
C'est encore un détail, mais c'est juste pour être rigoureux.

Enfin, je trouve dommage d'utiliser des tableaux pour accéder à une variable de langue. Même si c'est certainement très performant, je préfère la facilité d'écriture pour une perte de performance franchement dérisoire. Personnellement, j'utiliserais une classe supplémentaire, LangCat dont les propriétés (virtuelles, accessibles grâce à la méthode magique __get() ) seraient les variables de langue, dans la catégorie en question. On pourrait alors avoir une syntaxe comme celle-ci :

$lang = new Lang();
echo $lang -> cat -> var;

Autre chose : tu ne gères pas les erreurs. Que se passe-t-il si une langue n'existe pas ? A priori, d'après ce que je comprends du code, aucune clé de langue ne sera disponible.
Que se passe-t-il si la clé de langue n'existe pas ? Comme tu utilises des tableaux auxquels on accède directement depuis le code, on obtient une erreur. Il serait peut-être bon de retourner une valeur par défaut (au moins la clé demandée). Ceci serait possible avec une classe LangCat (qui contrôlerait l'accès aux variables de langue), mais ne l'est pas avec un accès direct au tableau.
Que se passe-t-il si dans le fichier de langue, une phrase contient un signe égal = ? La partie après le signe égal n'est pas prise en compte à cause de preg_split(). D'où l'intérêt de l'utilisation de la fonction parse_ini_file()

Une dernière chose (pour le moment ^^) cette classe est sympa, mais oblige à utiliser une syntaxe de type INI. Ce n'est pas dramatique, mais il pourrait être sympa de permettre l'utilisation de différents modes de stockage : INI, tableau PHP, XML, base de données, etc.
Pour cela, une classe générique qui gère la langue, le format de fichier (une factory), et des classes spécialisées pour chaque type. La factory aurait en charge d'ouvrir le fichier, d'en vérifier le format s'il n'est pas spécifié manuellement, et d'instancier la classe adéquate en fonction du type et de la retourner.
$lang = Lang::open('fichier.xml', LANG::XML);
$lang = Lang::open('autre_fichier.php);
Ce n'est qu'une suggestion.
Et pourquoi pas, proposer des fichiers gettext ?

Autre proposition : permettre la création de fichiers de langue, permettant ainsi de développer aisément une interface d'admin pour traduire dans le back-office ou convertir des fichiers d'un format à l'autre. Là, ça devient plus élaboré. Dans la même veine, proposer une conversion depuis un fichier Excel et/ou CSV (j'ai été confronté à ce problème récemment : le client voulait pouvoir traduire dans un tableur : je récupère le fichier XLS, je l'exporte en CSV et je le passe dans moulinette pour en faire un fichier avec un tableau PHP).

Sinon, je le répète : la classe est bien structurée et le code clair et facile à comprendre. Ca fait plaisir de voir un code comme ça ^^

Commentaire de CrazyShooter le 22/12/2009 11:41:14

Un petit problème avec la fonction parse_ini_file est qu'il faut mettre des guillemets pour les variables contenant des caractères spéciaux... avec ma fonction non par contre...