[spip-dev] Abandon de l'écriture $GLOBALS['meta']['x'] ? (crée des notices PHP à profusion !)

Dev
1

Bonjour à tous,

En corrigeant des indéfinis et notices PHP sur le trunk, je suis souvent confronté à une écriture casse pied avec les métas.

Très souvent, elles sont appelées sans test tel que
$x = $GLOBALS['meta']['x'];

Ceci est pour la plupart du temps correct. Sauf si… 'x' n'existe pas encore (installation) ou a été supprimée pour d'autres raisons.

L'écriture correcte devient :

$x = isset($GLOBALS['meta']['x']) ? $GLOBALS['meta']['x'] : null;

Tout de suite plus lourd.

Je propose qu'on charge systématiquement inc/config pour avoir accès à la fonction lire_config().

Ce qui donnerait par exemple
$charset = lire_config('charset', _DEFAULT_CHARSET);

ou $x = lire_config('x'); // $x null si la meta 'x' est non définie.

Ça me semble alléger ces écritures alambiquées.
Qu'en pensez vous ?

Ça rejoint aussi un peu http://blog.smellup.net/spip.php?article45

MM.

2

C'est quand même beaucoup plus lent de faire lire_config() qu'une simple lecture en globale, qui plus est que lire_config va tenter un unserialize() systématique.
A défaut, utiliser la fonction lire_meta() serait un moindre mal, mais ce serait revenir en arrière puisqu'elle existait et il avait été choisi de la remplacer par la lecture direct de la globale pour des raisons de perf.

Cédric

3

Mouais… enfin pour 1 appel de fonction… sinon lire_config('charset', _DEFAULT_CHARSET, false) ne tente pas de désérialiser non plus.

L'avantage autre que je vois à *_config() c'est que ça centralise la gestion des configurations. lire_config('/autre_table_meta/truc') est compris aussi.

Enfin en tout cas moi ça me casse les pieds d'écrire d'innombrables if (isset($GLOBALS['meta'][x]) ...

Ça me parait pas propre du tout.

MM.

4

Je suis plutôt d'accord avec Matthieu, à la fois pour des questions de clarté du code, mais aussi (et surtout) pour des questions d'abstraction !

Appeler directement des globales, je ne trouve pas ça propre, et si un jour on change de manière de stocker les configurations, il faudra tout reprendre. On pourra bien sûr faire un sed ou passer un par un, mais c'est un peu nul.

C'est le même principe que les lectures directes dans les variables de session à la SPIP, alors qu'on a à disposition une API de deux fonctions pour lire/écrire dans les sessions. Et que ça permet là aussi d'être certain que si on change de manière de stocker les sessions (pour utiliser celles de PHP ou autre), et bien le code restera le même.

Je pense que les optimisations doivent se faire au maximum, mais pas au dépend de la lisibilité et des bonnes pratiques de codage. Sinon on peut aussi écrire en assembleur : ça sera très rapide.

5

Je viens de tester quelques itérations pour voir.

Je parcours toute la table SPIP meta (104 clés):

Il y a effectivement une différence d'exécution non négligeable.

Pour un temps t en lisant directement la globale,
- un lire_meta() est 5 fois plus lent
- un lire_config() est 20 fois plus lent
- un lire_config(,false) est 10 fois plus lent.

Mais cette rapidité perdue ici est négligeable tout de même par rapport aux temps d'exécution d'autres fonctions de SPIP, comme par exemple couper() dans mon exemple, qui s'exécute 360 fois plus lentement.

Alors… je sais pas, mais tout de même…

1 fois :

Pour 104 opérations

     0.112 ms
     $x = isset($GLOBALS['meta'][$c]) ? $GLOBALS['meta'][$c] : null;

     0.356 ms
     $x = lire($c);
     (qui fait return isset($GLOBALS['meta'][$c]) ? $GLOBALS['meta'][$c] : null;) )

     1.318 ms
     $x = lire_config($c)

     0.707 ms
     $x = lire_config($c, null, false)

     26.380 ms
     $x = couper($c, 50)

100 fois :

Pour 10400 opérations

     7.122 ms
     $x = isset($GLOBALS['meta'][$c]) ? $GLOBALS['meta'][$c] : null;

     37.066 ms
     $x = lire($c);
     (qui fait return isset($GLOBALS['meta'][$c]) ? $GLOBALS['meta'][$c] : null;) )

     142.062 ms
     $x = lire_config($c)

     66.922 ms
     $x = lire_config($c, null, false)

     2 576.978 ms
     $x = couper($c, 50)