Guidelines de développement PHP
Introduction
La pratique de la revue de code est répandue chez Epiconcept. Cet article à pour but d’homogénéiser les pratiques des différentes équipes sur le sujet.
Les guidelines ci-dessous sont issues de réunions. Elles n’ont pas l’ambition d’être des vérités absolues. Leur objectif est simplement de s’assurer qu’une même pratique sera acceptée ou refusée de la même manière dans toutes les équipes lors de la revue de code.
Guidelines sur les commentaires
Les PHPDoc de fichiers, classes et méthodes
On parle de ça par exemple :
Ces commentaires servent à l’autocomplétion dans les IDE.
Les commentaires dans le code
On parle de ça par exemple :
Le code commenté
On parle de ça par exemple :
Soit le code est utile et doit être décommenté, soit il ne l’est pas et il doit être supprimé. Le laisser commenté laisse planer un doute. Et de plus, on a un outil de gestion de version pour gérer les suppressions de code et leur revert si nécessaire.
Guidelines sur les errorlog
On parle de ça :
On distingue trois types de logs :
- Les logs d’erreurs : issus du catch d’une exception par exemple
- Les logs d’activités : plus métiers, dédiés au suivi des activités stratégiques des utilisateurs (ex : connexion, modification d’un enregistrement, exécution d’une fonctionnalité très gourmande en ressources, etc.)
- Les logs de debug : utiles pendant le développement mais pas sur les serveurs
Les logs d’erreur
Exemple :
Les logs d’activités
Les logs de debug
Guidelines sur les @todo
On parle de ça :
En effet :
- Si le @todo n’est pas compréhensible, il faut le retirer.
- Si le @todo est dans le périmètre du ticket, il faut faire le développement avant de faire la PR (car laisser un @todo voudrait dire que le travail n’est pas terminé).
- Si le @todo est hors du périmètre du ticket, il faut créer un ticket autre Jira pour que ce soit planifié.
Guidelines sur l’usage des constantes
On parle de ça :
Elles permettent d’éviter de répéter une même valeur plein de fois dans le code (risque d’erreur et manque de factorisation).
Il n’y a pas de guidelines claires sur la préférence à avoir entre des constantes comme celles-ci et l’utilisation de propriétés de classe, mais quoi qu’il en soit :
Guidelines sur les méthodes statiques
On parle de ça :
Ces méthodes permettent de ne pas instancier leur classe, mais sont souvent un raccourci vers de la conception “moins” orientée objet. Elles peuvent être justifiées par des questions de performances ou de lisibilité du code, mais ne sont globalement pas recommandées.
Plus précisement :
Guidelines sur l’usage des classes de Zend pour l’accès aux données
On parle de ça :
Cette façon de procéder, que ce soit pour un INSERT, un UPDATE, ou autre, est globalement interdite.
En effet, contrairement aux méthodes du noyau comme SimpleUpdateData()
, elle présente deux problèmes :
- les droits de l’utilisateur courant ne sont pas appliqués ;
- les actions faites de cette manière ne sont pas tracées dans les tables
evtlog
etvarsetmonitor
de Voozanoo 4.
Or, ces deux points sont des engagements que nous avons vis-à-vis de nos clients et des autorités.
Ainsi :
Db()->update()
et ses soeurs sera globalement refusées en revue de code.SimpleUpdateData()
, il faut éviter de passer les champs NULL
en paramètres pour ne pas surcharger la table varsetmonitor
pour rien.