POO : est-ce que ce pattern est "propre" ?

Hey,

Si on imagine une class "Order" qui représente une commande e-commerce ; cette classe exploite une classe "Packaging" qui représente l’emballage à utiliser pour expédier cette commande (le carton ou l’enveloppe adéquate par exemple).

class Order {

private $packagingId;

public function getPackaging() : Packaging
{
  return new Packaging($this->packagingId);
}

}

Est-il pertinent de "mettre en cache" le Packaging au sein d’un attribut de la classe, pour l’utiliser plus rapidement les prochaines fois ?

class Order {

private $packagingId;
private $packaging;

public function getPackaging() : Packaging
{
  if (is_null($this->packaging)) {
    $this->packaging = new Packaging($this->packagingId);
  }
 
  return $this->packaging;
}

}

Cet exemple me pose 2 grandes questions :

1) Je me demande si cette idée de "mise en cache" est une bonne pratique ou non.

2) Plus largement, je me demande si la manière d’utiliser et d’imbriquer ces 2 class (Order et Packaging dans l’exemple) est "POO compliant" ou si je m’y prends mal. J’ai parfois l’impression qu’il faudrait mieux faire autrement, mais je ne vois pas comment.

Je me torture un peu là-dessus parce que j’aimerais améliorer mon code OO.

Merci d’avance pour vos lumières !

Mettre en cache des données dans un attribut n’est pas une mauvaise idée en soi, si tu as de bonnes raisons de perf derrière.

Ici dans ton cas le packaging devrait être crée dans le constructeur de la classe Order et non dans le getter.

Après je pense que ta relation Order et Packaging est mauvaise. Tout dépend de ta problématique réelle mais je peux imaginer une commande sans packaging et je ne suis pas sûr que ce soit une information essentielle à ta commande.

Le mieux serait sans doute d’avoir une classe (à définir) qui rassemblerait un Order et un Packaging ensemble. Mais sans plus de détails concernant l’application c’est difficile à se projeter.

Hey @Renault,

Merci pour ton retour ! (NB : j’ai modifié mon code dans le premier message, car mon système de cache était foireux, j’avais mal relu mon message – mais je ne pense pas que ça change ta réponse).

En mettant le chargement de Packaging::class dans le constructeur, j’ai l’impression de potentiellement consommer des ressources pour rien. Et si l’utilisateur de la class n’utilisait pas getPackaging() si souvent que ça ? On l’aurait chargé pour rien. C’est pour ça que j’ai pris l’habitude de charger certains objets dans le getter.

Dernière question concernant ta remarque "avoir une classe (à définir) qui rassemblerait un Order et un Packaging ensemble".

En fait, une Order::class doit forcément se voir associer un Packaging::class. Partant de là, quelle structure tu proposerais (vulgairement) pour relier les deux class ? Un truc du genre :

class GlobalOrder {

private $order; // Order::class
private $packaging; // Packaging:class

}

?

Merci ! bonne journée !

En fait, une Order::class doit forcément se voir associer un Packaging::class.

Peut-être n’est-ce pas le cas pour toi, mais souvent une commande a une vie plus ou moins longue dans le système avant qu’un emballage lui soit associé. Par exemple si un des items est en rupture de stock, ça peut être malin de prévoir l’envoi en plusieurs fois.

C’est un peu ce que je lis entre les lignes lorsque tu dis ça :

En mettant le chargement de Packaging::class dans le constructeur, j’ai l’impression de potentiellement consommer des ressources pour rien. Et si l’utilisateur de la class n’utilisait pas getPackaging() si souvent que ça ?

Je rejoins Renault, a priori ton Order n’a pas à avoir la responsabilité de son Packaging.

Pour la troisième classe, oui ça pourrait être quelque chose comme ça en effet. Elle pourrait s’appeller "Shipment" éventuellement, mais comme dit Renault, difficile d’en dire plus sans connaître les besoins.

Salut !

Est-il pertinent de "mettre en cache" le Packaging au sein d’un attribut de la classe, pour l’utiliser plus rapidement les prochaines fois ?

Ça dépend de tes besoins. Si tu as souvent recourt au packaging, alors oui. Sinon, peut-être pas ?

Le meilleur exemple que j’ai c’est dans l’utilisation d’un ORM (Doctrine pour PHP ?). J’ai des données « mappées » avec une base de données relationnelle, mais lorsque j’appelle des sous-attributs de sous-attributs et qu’ils ne sont pas chargés, ceux-ci sont mis en cache à chaque fois que je les utilise au moins une fois (et pour une utilisation future également).

En fait, une Order::class doit forcément se voir associer un Packaging::class.

Sans détour, assigne un attribut Packaging à un Order dans ce cas. Et si tu flippes d’une évolution future où un Order n’en a plus besoin, couple ton Order à une interface Packageable ou tu passes un peu de temps dessus pour pas trop te merder sur la conception, peut-être ?

Au plaisir d’en discuter en détail avec toi si l’occasion se présente.

Ça fait longtemps que je n’ai pas fait de PHP, mais je suppose que c’est comme dans les autres langages objet :

class Order {

private $packagingId;

public function getPackaging() : Packaging
{
  return new Packaging($this->packagingId);
}

}

Ici, je vois deux « mauvaises pratiques », ou plus exactement des pratiques qui vont à l’encontre des conventions :

1. Normalement une méthode getMachin() renvoie directement un machin qui est déjà connu ou trivial à calculer, et donc qui sera très rapide. Or, visiblement, ici tu charges les données du packaging à partir de son ID, ce qui n’est pas l’un de ces cas. Il vaudrait mieux utiliser un autre verbe, en fonction de comment tu calcules le packaging.
2. De la même manière, new Packaging($this->packagingId) laisse entendre que tu construits un packaging à partir de son ID dans le constructeur. Selon comment tu récupères les données, ça peut mener à des calculs longs ou complexes à l’intérieur du constructeur. Or il vaut mieux laisser ces derniers les plus simples possibles, pour des raisons de performances et de comportement de gestion des erreurs dans les constructeurs.

Ça a l’air d’être du pinaillage, mais pas tant que ça : ce genre de pratiques peut rapidement mener à du code avec de gros problèmes de performance (typiquement si un développeur utilise le getter en pensant que c’est une méthode rapide alors que non), ou des erreurs particulièrement pénibles à gérées parce que lancées depuis l’intérieur d’un constructeur.

Bonsoir,

Ca fait aussi un moment que j’ai plus fait de PHP, mais je suis d’accord avec Spacefox.

En fait, le chargement d’un objet Packaging à partir de son ID, ça devrait être le travail d’une factory. Dans le langage des ORM, on appelle ça un repository.

C’est assez maladroit de faire ça dans le constructeur, car du coup tu te couples fortement avec la source de donnée. Tu devras peut-être prendre des contours pour créer des objets non (encore) enregistrés, et pour faire des tests unitaires ça va être très très compliqué.

De façon générale, le constructeur est censé rester assez simple, comme les getters/setters.

Salut @Ge0 Merci pour ton retour ! Je vais réfléchir à ce que tu as dit, ça semble bien intéressant !

@SpaceFox @QuentinC Merci beaucoup pour vos explications, ce sont exactement les réponses qu’il me manquait pour clarifier la situation. Je vais pouvoir partir de là pour démarrer plus proprement mon prochain projet.

Dernière question générale : quel cours/tuto/vidéo/formation/livre recommanderiez-vous à quelqu’un pour améliorer sa conception en POO ?

J’ai consommé pas mal de ressources sur les design pattern les plus pratiques, et c’était déjà une bonne étape pour mieux structurer mon code. Mais certaines pratiques basiques (comme celles que j’ai soulevées dans ce thread) sont rarement documentées.

Merci à tous pour votre aide ! Bon dimanche.

J’aime assez Tête la première — Design Patterns de Eric & Elisabeth Freeman perso.

Ensuite :

Mais certaines pratiques basiques (comme celles que j’ai soulevées dans ce thread) sont rarement documentées.

C’est parce qu’on ne peut pas documenter tous les cas concrets de problème. En général avec l’expérience tu arrives à dire toi-même si la solution que tu utilises est bonne ou non, en fonction de la situation.

Par exemple, j’ai l’avantage de te connaître et je sais que tu ne travailles pas dans une grande équipe. En ce sens, je ne t’ai pas suggéré de faire ce que SpaceFox et QuentinC ont pu te suggérer, car je ne l’ai pas jugé nécessaire sur l’instant.

Écrire du code maintenable à 100 % cela fonctionne uniquement dans le meilleur des mondes. Idéalement on s’y prête également quand on travaille seul, mais là encore tout le monde ne peut pas prétendre à avoir 10 ans d’expérience.

Donc, j’ai envie de te dire : pas tellement besoin de te prendre la tête, si ce n’est : écris du code que tu es capable de comprendre toi, disons, 6 mois plus tard.

Dernière question générale : quel cours/tuto/vidéo/formation/livre recommanderiez-vous à quelqu’un pour améliorer sa conception en POO ?

A mon avis, les design patterns et les bonnes pratiques ne sont pas immuables, et tous les livres ou tutoriels que tu pourras lire sur le sujet ne font que’effleurer des bases communes et évidentes sur des exemples typiques. C’est instructif bien sûr, mais pas suffisant. La vraie réponse, je dirais, c’est que ça se forge principalement avec l’expérience, sur différents projets, avec différentes équipes, et que dans la pratique, il n’y a jamais de réponse totalement tranchée sur tout.