Est-ce une bonne idée de définir une grande fonction privée dans une classe pour maintenir un état valide, c'est-à-dire pour mettre à jour les membres de données de l'objet?

18

Bien que dans le code ci-dessous, un simple achat d'un article dans un site de commerce électronique soit utilisé, ma question générale concerne la mise à jour de tous les membres de données pour garder les données d'un objet dans un état valide à tout moment.

J'ai trouvé «cohérence» et «état est mauvais» comme expressions pertinentes, discutées ici: https://en.wikibooks.org/wiki/Object_Oriented_Programming#.22State.22_is_Evil.21

<?php

class CartItem {
  private $price = 0;
  private $shipping = 5; // default
  private $tax = 0;
  private $taxPC = 5; // fixed
  private $totalCost = 0;

  /* private function to update all relevant data members */
  private function updateAllDataMembers() {
    $this->tax =  $this->taxPC * 0.01 * $this->price;
    $this->totalCost = $this->price + $this->shipping + $this->tax;
  }

  public function setPrice($price) {
      $this->price = $price;
      $this->updateAllDataMembers(); /* data is now in valid state */
  }

  public function setShipping($shipping) {
    $this->shipping = $shipping;
    $this->updateAllDataMembers(); /* call this in every setter */
  }

  public function getPrice() {
    return $this->price;
  }
  public function getTaxAmt() {
    return $this->tax;
  }
  public function getShipping() {
    return $this->shipping;
  }
  public function getTotalCost() {
    return $this->totalCost;
  }
}
$i = new CartItem();
$i->setPrice(100);
$i->setShipping(20);
echo "Price = ".$i->getPrice(). 
  "<br>Shipping = ".$i->getShipping().
  "<br>Tax = ".$i->getTaxAmt().
  "<br>Total Cost = ".$i->getTotalCost();

Des inconvénients ou peut-être de meilleures façons de le faire?

Il s'agit d'un problème récurrent dans les applications du monde réel soutenu par une base de données relationnelle, et si vous n'utilisez pas de procédures stockées de manière extensive pour pousser toute la validation dans la base de données. Je pense que le magasin de données devrait simplement stocker des données, tandis que le code devrait faire tout le travail de maintenance de l'état d'exécution.

EDIT: c'est une question connexe mais n'a pas de recommandation de meilleures pratiques concernant une seule grande fonction pour maintenir un état valide: /programming/1122346/c-sharp-object-oriented-design-maintaining- état-objet-valide

EDIT2: Bien que la réponse de @ eignesheep soit la meilleure, cette réponse - /software//a/148109/208591 - est ce qui remplit les lignes entre la réponse de @ eigensheep et ce que je voulais savoir - le code ne devrait être traité que, et l'état global doit être remplacé par un passage d'état activé par DI entre les objets.

site80443
la source
J'évite d'avoir des variables qui sont des pourcentages. Vous pouvez accepter un pourcentage de l'utilisateur, ou afficher un à un utilisateur, et la vie est beaucoup mieux si les variables du programme sont des rapports.
kevin cline

Réponses:

29

Toutes choses étant égales par ailleurs, vous devez exprimer vos invariants en code. Dans ce cas, vous avez l'invariant

$this->tax =  $this->taxPC * 0.01 * $this->price;

Pour exprimer cela dans votre code, supprimez la variable de membre fiscal et remplacez getTaxAmt () par

public function getTaxAmt() {
  return $this->taxPC * 0.01 * $this->price;
}

Vous devriez faire quelque chose de similaire pour vous débarrasser de la variable membre de coût total.

Exprimer vos invariants dans votre code peut aider à éviter les bogues. Dans le code d'origine, le coût total est incorrect s'il est vérifié avant l'appel à setPrice ou setShipping.

eigensheep
la source
3
De nombreuses langues ont getters afin que ces fonctions ne prétendre qu'ils sont des propriétés. Le meilleur des deux!
curiousdannii
Excellent point, mais mon cas d'utilisation générale est lequel les données récupère et stocke le code à plusieurs colonnes dans plusieurs tables dans une base de données relationnelle (MySQL principalement) et je ne veux pas utiliser des procédures stockées (discutable et un autre sujet sur lui - même). Prendre votre idée en invariants autre code cela signifie que tous les calculs doivent être « enchaînés »: getTotalCost()appels getTaxAmt()et ainsi de suite. Cela signifie que nous ne stockent que jamais les êtres non calculés . Avançons - nous un peu vers la programmation fonctionnelle? Cela complique également le stockage des entités calculées dans des tableaux pour un accès rapide ... A besoin d'expérimentation!
site80443
13

Des inconvénients [?]

Sûr. Cette méthode repose sur tout le monde se souvenant toujours de faire quelque chose. Toute méthode compter sur tout le monde et est toujours vouée à l' échec parfois.

peut-être de meilleures façons de le faire?

Une façon d'éviter le fardeau de se souvenir de la cérémonie est de calculer les propriétés de l'objet qui dépendent d'autres propriétés selon les besoins, comme l'a suggéré @eigensheep.

Une autre consiste à rendre l'élément du panier immuable et à le calculer dans le constructeur / la méthode d'usine. Vous devriez normalement utiliser la méthode "calculer au besoin", même si vous avez rendu l'objet immuable. Mais si le calcul prend trop de temps et serait lu plusieurs fois; vous pouvez choisir l'option "calculer lors de la création".

$i = new CartItem();
$i->setPrice(100);
$i->setShipping(20);

Vous devriez vous demander; Un article de panier sans prix est-il logique? Le prix d'un article peut-il changer? Après sa création? Après sa taxe calculée? etc Peut - être que vous devriez faire CartItemPrix immuable et Affect et l' expédition dans le constructeur:

$i = new CartItem(100, 20);

Est-ce qu'un article du chariot du sens sans le panier auquel il appartient?

Sinon, je m'attendrais à la $cart->addItem(100, 20)place.

abuzittin gillifirca
la source
3
Vous signalez le plus gros inconvénient: compter sur des personnes qui se souviennent de faire des choses est rarement une bonne solution. La seule chose sur laquelle vous pouvez compter sur un humain, c'est qu'il oubliera de faire quelque chose.
corsiKa
@corsiKlause Ho Ho Ho et abuzittin, solide point, ne peut pas dire le contraire - les gens oublient toujours . Cependant, le code que j'ai écrit ci-dessus n'est qu'un exemple, il existe des cas d'utilisation substantiels où certains membres de données sont mis à jour plus tard. L'autre façon que je vois est de normaliser davantage - faire des classes telles que les membres de données mis à jour indépendamment sont dans d'autres classes et pousser la responsabilité de la mise à jour sur certaines interfaces - de sorte que d'autres programmeurs (et vous-même après un certain temps) doivent écrire une méthode - le le compilateur vous rappelle que vous devez l'écrire. Mais cela ajouterait beaucoup plus de classes ...
site80443
1
@ site80443 Sur la base de ce que je vois, qui est une mauvaise solution. Essayer de modéliser vos données telles que seules les données qui est validée par comparaison avec lui - même est inclus. Par exemple , le prix d'un article ne peut pas être négatif ne repose que sur lui - même. Si un article est réduit, ne tenez pas compte de la remise dans le prix - décorez-le avec une remise plus tard. Conservez les 4,99 $ pour l'article et la remise de 20% en tant qu'entité distincte, et la taxe de 5% sur une autre entité. Il semble que vous devriez considérer le modèle Decorator si les exemples représentent votre code réel.
corsiKa