En effet, le test correspondant à la méthode utilisant ce code passait au rouge de manière complètement aléatoire.
J’ai donc cherché l’origine de ce dysfonctionnement et après m’être tapé la tête dans les murs à plusieurs reprises, j’ai fini par comprendre.
Dans mon test unitaire, j’utilise la fonction uniqid() pour nourrir la méthode que je teste.
Comme son nom l’indique, cette fonction génère une chaîne aléatoire unique, composée de lettres et de chiffres, y compris le chiffre 0.
Or, si le résultat de l’appel à la fonction substr() est 0, la condition à gauche de l’opérateur ?: n'est pas vérifiée et ce dernier retourne donc la valeur située à sa droite.
Du coup, un caractère est perdu par mon code, ce qui fait alors passer mon test au rouge.
La solution consiste donc à recourir à une solution plus traditionnelle pour tester le retour de la fonction substr(), au prix d’un code moins condensé, à savoir $string = substr($string, $index); if ($string === false) $string = '';.
Et évidemment, il faut également ajouter dans le test unitaire le cas ou substr() retourne la chaîne 0 afin de prévenir l’éventuelle régression susceptible de survenir lorsque, dans quelque temps, j’aurais oublié la raison d’être de ce formalisme plus verbeux ou lorsqu’un tiers voudra le remplacer par l’opérateur ?: en pensant bien faire.
Et c’est également l’un des rares cas ou l’ajout d’un commentaire dans le code me semble pertinent.
Et pour finir, la façon dont j'ai découvert ce bug démontre l'intérêt de recourir à l'aléatoire via par exemple uniqid() dans les tests, car sans cela, j'aurais pu passer à côté et le découvrir bien plus tard, à un moment où il aurait été potentiellement bien plus onéreux à corriger.
7 réactions
1 De Guile - 28/11/2013, 17:14
Son effet si limitant me pousse à chaque fois à éviter l'opérateur elvis... pourtant ce n'est pas l'envie qui manque... mais à chaque fois que je pense à tous les cas possibles, je le remplace par une solution plus verbeuse...
2 De bdelespierre - 28/11/2013, 17:18
J'ai vu bien pire mais celle là était pas mal
btw, on peut toujours le faire en une seule instruction (moins lisible pour le coup):
$string = ($tmp = substring($string, $index)) !== false ? $tmp : '';
3 De jpicaude - 28/11/2013, 17:55
Et en augmentant simplement la longueur retournée par substr ?
A moins de n'avoir vraiment que des 0 qui s'affichent, auquel cas, il faut songer sérieusement à jouer au loto, tu pourrais aussi retrouver ta green bar, non ?
4 De Anvi - 29/11/2013, 07:15
Dans le cas presenté, ne pourrait-on pas ecrire :
$string = (string) substr($string, $index);
5 De Antwan - 29/11/2013, 10:44
6 mois je fais du python (vs 10ans de php) et ce genre d'article me fait de plus en plus dire que je n'écrirai plus jamais une ligne de ça. #trolldi
6 De mageekguy - 29/11/2013, 11:23
@Antwan : j'aimerais beaucoup que ce soit le cas, mais malheureusement, le langage n'est pas responsable de ma bêtise.
Dans le cas contraire, je m'empresserais de faire une PR sur le dépôt github de PHP résolvant le bug ;).
La façon dont PHP compare les types et est parfaitement documentée, et je me suis laissé emporté par mes habitudes et mon enthousiasme plutôt que de prendre le temps de réfléchir deux minutes à ce que je faisais.
Si un coupable doit être désigné, c'est donc l'interface entre la chaise et le clavier.
7 De Nash - 29/11/2013, 18:10
Bien vu ! et aussi pour l'exploitation de uniqid()
Par contre, personnellement, je trouve que l'instruction manque de clarté par rapport au but recherché, plus pour substr() que pour l'opérateur ternaire d'ailleurs. Il faut vraiment connaitre le détail de fonctionnement de substr() pour l'exploiter pour le cas indiqué (j'aurais utilisé quelque chose de plus verbeux...).