Ist es eine gute Idee, eine große private Funktion in einer Klasse zu definieren, um den gültigen Status beizubehalten, dh die Datenelemente des Objekts zu aktualisieren?

18

Obwohl im folgenden Code ein einfacher Kauf eines einzelnen Artikels auf einer E-Commerce-Website verwendet wird, geht es bei meiner allgemeinen Frage darum, alle Datenelemente zu aktualisieren, um die Daten eines Objekts jederzeit in einem gültigen Zustand zu halten.

Ich fand "Konsistenz" und "Zustand ist böse" als relevante Ausdrücke, die hier besprochen wurden: 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();

Irgendwelche Nachteile oder vielleicht bessere Möglichkeiten, dies zu tun?

Dies ist ein immer wiederkehrendes Problem in realen Anwendungen, die von einer relationalen Datenbank unterstützt werden, und wenn Sie gespeicherte Prozeduren nicht ausgiebig verwenden, um die gesamte Validierung in die Datenbank zu übertragen. Ich denke, dass der Datenspeicher nur Daten speichern sollte, während der Code die gesamte Arbeit zur Aufrechterhaltung des Laufzeitstatus erledigen sollte.

BEARBEITEN: Dies ist eine verwandte Frage, es gibt jedoch keine Best-Practice-Empfehlung für eine einzelne große Funktion, um den gültigen Status beizubehalten: /programming/1122346/c-sharp-object-oriented-design-maintaining- Gültiger Objektstatus

EDIT2: Obwohl die Antwort von @ eignesheep die beste ist, füllt diese Antwort - /software//a/148109/208591 - die Grenzen zwischen der Antwort von @ eigensheep und dem, was ich wissen wollte - Code sollte nur verarbeitet werden. Der globale Status sollte durch die DI-aktivierte Statusübergabe zwischen Objekten ersetzt werden.

site80443
quelle
Ich vermeide Variablen, die Prozentsätze sind. Sie können einen Prozentsatz vom Benutzer akzeptieren oder einem Benutzer anzeigen, aber das Leben ist viel besser, wenn die Programmvariablen Verhältnisse sind.
Kevin Cline

Antworten:

29

Wenn alles andere gleich ist, sollten Sie Ihre Invarianten in Code ausdrücken. In diesem Fall haben Sie die Invariante

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

Um dies in Ihrem Code auszudrücken, entfernen Sie die Steuervariable und ersetzen Sie getTaxAmt () durch

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

Sie sollten etwas Ähnliches tun, um die Variable "Gesamtkostenelement" zu entfernen.

Das Ausdrücken Ihrer Invarianten in Ihrem Code kann helfen, Fehler zu vermeiden. Im Originalcode sind die Gesamtkosten falsch, wenn sie vor dem Aufruf von setPrice oder setShipping überprüft werden.

Eigenschaf
quelle
3
Viele Sprachen haben Getter, so dass solche Funktionen so tun, als wären sie Eigenschaften. Das Beste von beidem!
curiousdannii
Hervorragender Punkt, aber mein allgemeiner Anwendungsfall ist, dass Code Daten abruft und in mehreren Spalten in mehreren Tabellen in einer relationalen Datenbank speichert (meistens MySQL) und ich keine gespeicherten Prozeduren verwenden möchte (umstritten und ein anderes Thema für sich). Wenn Sie Ihre Idee der Invarianten im Code weiter verfolgen, bedeutet dies, dass alle Berechnungen "verkettet" werden müssen: getTotalCost()Aufrufe getTaxAmt()und so weiter. Das heißt, wir speichern immer nur nicht kalkulierte Dinge . Bewegen wir uns ein bisschen in Richtung funktionale Programmierung? Dies erschwert auch die Speicherung von berechneten Entitäten in Tabellen für den schnellen Zugriff.
Site80443
13

Nachteile [?]

Sicher. Diese Methode beruht darauf, dass sich jeder daran erinnert , etwas zu tun. Jede Methode, die sich auf alle stützt, ist manchmal zum Scheitern verurteilt .

vielleicht bessere Möglichkeiten, dies zu tun?

Eine Möglichkeit, die Last des Erinnerns an die Zeremonie zu vermeiden, besteht darin, die Eigenschaften des Objekts zu berechnen, die nach Bedarf von anderen Eigenschaften abhängen, wie von @eigensheep vorgeschlagen.

Zum anderen wird der Warenkorbartikel unveränderlich gemacht und im Konstruktor / in der Factory-Methode berechnet. Normalerweise würden Sie die Methode "Berechnen nach Bedarf" verwenden, selbst wenn Sie das Objekt unveränderlich machen würden. Aber wenn die Berechnung zu zeitaufwändig ist und viele, viele Male gelesen werden würde; Sie können die Option "Beim Erstellen berechnen" wählen.

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

Sie sollten sich fragen; Ist ein Einkaufswagenartikel ohne Preis sinnvoll? Kann sich der Preis eines Artikels ändern? Nachdem es erstellt wurde? Nach seiner Steuer berechnet? etc Vielleicht solltest du CartItemunveränderlichen und assig Preis und Versand im Konstruktor machen:

$i = new CartItem(100, 20);

Ist ein Warenkorbartikel ohne den dazugehörigen Warenkorb sinnvoll?

Wenn nicht, würde ich $cart->addItem(100, 20)stattdessen erwarten .

abuzittin gillifirca
quelle
3
Sie weisen auf den größten Nachteil hin: Sich darauf zu verlassen, dass Menschen daran denken, Dinge zu tun, ist selten eine gute Lösung. Sie können sich nur darauf verlassen, dass ein Mensch vergisst, etwas zu tun.
CorsiKa
@corsiKlause Ho Ho Ho und abuzittin, solid point, können sich nicht streiten - die Leute vergessen immer . Der Code, den ich oben geschrieben habe, ist jedoch nur ein Beispiel. Es gibt erhebliche Anwendungsfälle, in denen einige Datenelemente später aktualisiert werden. Die andere Möglichkeit, die ich sehe, besteht darin, weiter zu normalisieren - Klassen so zu machen, dass unabhängig aktualisierte Datenelemente zu anderen Klassen gehören, und die Verantwortung für die Aktualisierung auf einige Schnittstellen zu verlagern - damit andere Programmierer (und Sie selbst nach einiger Zeit) eine Methode schreiben müssen - die Der Compiler erinnert Sie daran, dass Sie es schreiben müssen. Aber das würde viel mehr Klassen hinzufügen ...
Site80443
1
@ site80443 Basierend auf dem, was ich sehe, ist das der falsche Ansatz. Versuchen Sie, Ihre Daten so zu modellieren, dass nur Daten enthalten sind, die für sich selbst validiert wurden. Zum Beispiel kann der Preis eines Artikels nicht negativ sein, sondern hängt von sich selbst ab. Wenn ein Artikel rabattiert ist, berücksichtigen Sie den Rabatt nicht im Preis - verzieren Sie ihn später mit einem Rabatt. Speichern Sie die 4,99 USD für den Artikel und den 20% -Rabatt als separate Einheit und die 5% -Steuer als weitere Einheit. Es sieht tatsächlich so aus, als sollten Sie das Decorator-Muster berücksichtigen, wenn die Beispiele Ihren realen Code darstellen.
corsiKa