E 'una buona idea definire una grande funzione privata in una classe per mantenere uno stato valido, vale a dire aggiornare i membri dei dati dell'oggetto?

Naviga le tue risposte
18

Sebbene nel codice sottostante venga utilizzato un semplice acquisto di un singolo articolo in un sito di e-commerce, la mia domanda generale riguarda l'aggiornamento di tutti i membri dei dati per mantenere i dati di un oggetto sempre in uno stato valido.

Ho trovato "coerenza" e "stato è cattivo" come frasi pertinenti, discusse qui: link 

<?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();

Eventuali svantaggi o forse modi migliori per farlo?

Si tratta di un problema ricorrente nelle applicazioni reali supportato da un database relazionale e, se non si utilizzano ampiamente le stored procedure, è necessario trasferire tutta la convalida nel database. Penso che l'archivio dati debba solo memorizzare i dati, mentre il codice dovrebbe fare tutto il lavoro di mantenimento dello stato di esecuzione.

EDIT: questa è una domanda correlata ma non ha una raccomandazione di best practice riguardante una singola grande funzione per mantenere uno stato valido: link

EDIT2: Sebbene la risposta di @ eignesheep sia la migliore, questa risposta - link - è ciò che riempie le linee tra @ La risposta di eigensheep e ciò che volevo sapere - il codice dovrebbe solo elaborare e lo stato globale dovrebbe essere sostituito dal passaggio di stato abilitato alla DI tra gli oggetti.

    
posta site80443 24.12.2015 - 07:37
fonte

2 risposte

29

A parità di tutti gli altri, dovresti esprimere i tuoi invarianti nel codice. In questo caso hai l'invariante 

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

Per esprimere questo nel tuo codice, rimuovi la variabile membro fiscale e sostituisci getTaxAmt () con 

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

Dovresti fare qualcosa di simile per sbarazzarti della variabile membro di costo totale.

Esprimere i tuoi invarianti nel tuo codice può aiutare a evitare i bug. Nel codice originale, il costo totale non è corretto se selezionato prima di impostare setPrice o setShipping.

    
risposta data 24.12.2015 - 08:27
fonte
12

Any disadvantages[?]

Certo. Questo metodo si basa su che tutti ricordano sempre di fare qualcosa. Qualsiasi metodo basato su tutti e sempre è destinato a fallire a volte

maybe better ways to do this?

Un modo per evitare l'onere di ricordare la cerimonia è calcolare le proprietà dell'oggetto che dipendono da altre proprietà secondo necessità, come suggerito da @eigensheep.

Un altro sta rendendo immutabile la voce del carrello e calcolandola nel costruttore / nel metodo di produzione. Normalmente si usa il metodo "calcola come necessario", anche se l'oggetto è immutabile. Ma se il calcolo richiede troppo tempo e verrebbe letto molte, molte volte; puoi scegliere l'opzione "calcola durante la creazione". 

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

Dovresti chiederti; Un articolo di carrello senza un prezzo ha un senso? Il prezzo di un articolo può cambiare? Dopo che è stato creato? Dopo aver calcolato la tassa? ecc. Forse dovresti rendere CartItem immutabile e assigare prezzo e spedizione nel costruttore: 

$i = new CartItem(100, 20);

L'articolo di un carrello ha senso senza il carrello a cui appartiene?

Altrimenti, mi aspetterei invece $cart->addItem(100, 20) .

    
risposta data 24.12.2015 - 13:39
fonte

Leggi altre domande sui tag

Quanto è accettabile il refactoring? [duplicare] Lingua funzionale più veloce