Magento 2 - bonne pratique pour utiliser / éviter les getters magiques?

21

Les getters magiques sur Varien_Object(M1) et DataObject(M2) sont une pratique courante, mais avec Magento 2, il ne convient pas de l'utiliser.

Bien:

  • facile à lire / écrire

Mal

Question

Avec Magento 2, nous avons deux nouvelles méthodes:

  • getDataByKey($key)
  • getDataByPath($path)

Y a-t-il une bonne raison d'utiliser encore getData($key)ou des getters magiques?

Éditer:

@Vinai merci. Je n'ai pas mentionné la @methodméthode, car mon approche était assez différente.

Cela n'aide que l'IDE, mais n'a aucun impact sur d'autres choses.

Il existe plusieurs PR fusionnés sur lesquels sont des «micro-optimisations», comme le transtypage en (int)lieu et place intval()ou la taille des tableaux en dehors des boucles (même pour les petits tableaux).

D'un autre côté, il y a

  1. des getters magiques, qui ont des "frais généraux" comme Marius l'a décrit ....

    strtolower(trim(preg_replace('/([A-Z]|[0-9]+)/', "_$1", $name), '_'));

  2. getData($key) les mehtods doivent également effectuer 2 à 3 contrôles supplémentaires ...

    • if ('' === $key) {
    • if (strpos($key, '/')) {
    • if ($index !== null) {

Pour votre propre code, vous êtes totalement d'accord pour préférer de vraies méthodes, mais dans les mêmes cas ce n'est pas possible ... par exemple, vous avez créé un événement personnalisé ...

$value = $observer->getVar_1();
$value = $observer->getData('var_1');
$value = $observer->getDataByKey('var_1');

Utiliser le 3e avec /** @var some $value */me semble le mieux. (?)

magento2 best-practice coding-standards code-analysis magic-getter sv3n
la source
1
Vous pouvez ajouter les méthodes dans le bloc doc de classe, afin que les outils d'analyse de code ne se plaignent pas des méthodes inexistantes. Je pense également que l'utilisation de chiffres dans les clés est en soi une mauvaise pratique, donc elle ne devrait pas être répertoriée comme "Bad" ici imo.
Lily Bergonzat

Réponses:

20

La question ci-dessus concerne l'utilisation de méthodes magiques contre getDataByKeyou getDataByPath. Je pense qu'il y a aussi une troisième option, qui met en œuvre de vraies méthodes getter et setter.

Les getData*méthodes ont toutes l'inconvénient de devoir être annotées pour que l'inférence de type fonctionne.
Habituellement, cela se fait avec une /* @var string $foo */annotation au-dessus de l' getData*appel.
C'est un peu malodorant, car le type des données doit être déclaré dans la classe qui contient les données, pas la classe qui appelle getData*.
La raison en est que si les données changent, la classe est la plus susceptible d'être mise à jour, pas tous getData*les sites d'appel.
C'est pourquoi je pense que les méthodes réelles augmentent la maintenabilité par rapport à l'utilisation d' getData*accesseurs.

Je pense donc que cela se résume à un compromis entre la maintenabilité et une implémentation plus rapide (moins de code à écrire).

Heureusement, de nos jours, les IDE sont vraiment bons pour créer les implémentations getter et setter pour nous, donc cet argument ne s'applique plus vraiment.

Un autre argument contre les getters et setters magiques qui manque dans la question ci-dessus est qu'il n'est pas possible de créer des plugins pour eux.

La seule autre valeur que je pense pouvoir ajouter au sujet est d'essayer de collecter les raisons d'utiliser ou de ne pas utiliser d' @methodannotations, si la mise en œuvre de méthodes réelles est hors de question pour une raison quelconque.

Avantages

  • Une @methodannotation est un peu moins de code à écrire que l'implémentation d'un vrai getter et setter. Cela est à peine vrai de nos jours car les IDE sont bons pour générer des méthodes d'accesseur, donc ce n'est plus un réel avantage.

Les inconvénients

  • Il est facile que les choses tournent mal.
    • Les annotations sont des commentaires, elles deviennent facilement obsolètes lorsque le code évolue mais les annotations ne sont pas mises à jour. Les méthodes réelles sont plus robustes.
    • Il est possible d'ajouter plusieurs annotations avec des signatures de type différent sans erreur d'interpréteur - le comportement d'analyse de code statique n'est pas défini, et il peut conduire à des bogues subtils difficiles à localiser.
    • S'il @methodexiste à la fois une annotation et une méthode réelle du même nom, la signature du type d'annotation remplace la méthode réelle lors de l'analyse de code statique, ce qui est l'opposé de ce que fait l'interpréteur PHP. Encore une fois, cela peut facilement conduire à des bugs subtils.

Pour les raisons ci-dessus, personnellement, je n'utilise pas d' @methodannotations si je peux les éviter.
Pour le code qui est destiné à vivre longtemps, j'implémente de vraies méthodes getter et setter. Le gain de maintenabilité vaut la peine de déclencher l'IDE pour les générer.

Pour du code plus expérimental lors d'un pic, ou pour un simple détail d'implémentation d'un module, j'utilise aussi des getData*méthodes, car je suis paresseux.

Vinai
la source
Joli résumé. Merci Vinai. Cela répond plus que je ne l'ai réellement demandé.
sv3n
1

Les getData*méthodes ont toutes l'inconvénient de devoir être annotées pour que l'inférence de type fonctionne.

Habituellement, cela se fait avec une /*@var string $foo */annotation au-dessus de l' getData*appel. C'est un peu malodorant, car le type des données doit être déclaré dans la classe qui contient les données, pas la classe qui appelle getData *.

La raison en est que si les données changent, la classe est la plus susceptible d'être mise à jour, pas tous getData*les sites d'appel. C'est pourquoi je pense que les méthodes réelles augmentent la maintenabilité par rapport à l'utilisation d'accesseurs getData *.

Oui, il sent mauvais, mais peut (et devrait?) Être évité. Je pense que c'est un code très courant et souvent suggéré:

/** @var Foo $product */
$product = $model->getProduct()
if ($product->getId()) {
    $product->doSomething();
}

Le problème est que vous devinez simplement que la valeur de retour est de type Fooavec une getId()méthode appelable .

Pour la maintenabilité, pourquoi ne pas assumer le type de variable et ajouter un InvalidArgumentException?

$product = $model->getProduct()
if ($product instanceof Foo && $product->getId()) {
    $product->doSomething();
}

Cela corrige également l'analyse de code statique dans le cas qui $model->getProduct()a différents types de retour - comme Foo|false. Dans le premier cas, il se plaindrait d'appeler doSomething()possible false.

sv3n
la source