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?

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.

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.

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.