Design della classe: i metodi dovrebbero chiamare altri metodi?

3

Sto scrivendo una classe personalizzata molto semplice per i coupon e ho trovato un layout di base per la classe, che consiste in una serie di piccoli metodi che, in generale, sono una best practice.

Ciò di cui non sono sicuro è come dovrebbe essere dettato il flusso di una classe. In questo momento il codice ha un aspetto simile al seguente:

public function isCouponValid($coupon) {
    if (strlen($coupon) != 8) return false;

    $query = $this->db->prepare('SELECT coupon_code FROM coupons WHERE coupon_code=:coupon');
    try ($query->execute(array(':coupon' => $coupon))) {
        while ($row = $query->fetch()) {
            return true;
        }   
    } catch (PDOException $e) {
        $log->addError('Could not execute SQL query: '.$query.' '.$e->getMessage());
    }

    return false;
}

public function isCouponUsed($coupon) {
    if (self::isCouponValid($coupon)) {
        $query = $this->db->prepare('SELECT coupon_used FROM coupons WHERE coupon_code=:coupon');
        try ($query->execute(array(':coupon' => $coupon))) {
            while ($row = $query->fetch()) {
                return ($row['coupon_used'] == '1') ? true : false;
            }   
        } catch (PDOException $e) {
            $log->addError('Could not execute SQL query: '.$query.' '.$e->getMessage());
        }
    }

    return false;
}

public function setCouponUsed($coupon) {
    if (self::isCouponValid($coupon) && !self::isCouponUsed($coupon)) {
        etc...
    }
}

Fondamentalmente ho questi controlli integrati nella classe adesso - isCouponUsed () chiama isCouponValid () per verificare che sia valido per primo. È una buona idea, o è meglio per me usare isCouponValid () al di fuori della classe e quindi passare quel valore a isCouponUsed (), tale che isCouponUsed () dovrebbe riguardare solo la convalida di un coupon e non controllare se quel codice è valido per primo?

    
posta Roy 08.12.2014 - 15:23
fonte

3 risposte

3

Penso che questo dipenda dall'incapsulamento.

Se devi offrire metodi pubblici per controllare Coupon, non puoi garantire che l'utente della tua API chiamerà isCouponValid prima di chiamare isCouponUsed , quindi devi aggiungere il controllo extra. Ma queste situazioni sono sempre discutibili e di solito suggerisce che il tuo design non segue principi come TellDon'tAsk .

Ma - e credo che questo sia il caso - se le convalide fanno parte del comportamento di un oggetto, come sembra suggerire il metodo setCoupon , allora puoi incapsularle come metodi privati (dettagli di implementazione) di l'oggetto. Non appena vengono incapsulati, l'oggetto ha il pieno controllo dell'ordine di esecuzione, quindi puoi scegliere di chiamare solo isCouponUsed quando il coupon è effettivamente valido, come già implementato nell'istruzione if di% co_de metodo%.

    
risposta data 08.12.2014 - 16:06
fonte
1

Dovresti preoccuparti delle classi che guidano il comportamento di altre classi. Questo significa essere cauti (non necessariamente evitare sempre) l'operatore punto.

Supponiamo che tu sia un ragazzo che consegna la pizza, e tu vieni a casa mia cercando di portarmi una pizza, hai intenzione di entrare nel mio portafoglio e prendere i miei soldi? Cosa succede se i miei soldi sono sul frigorifero? Questo è chiamato alto accoppiamento.

Significa che per programmare funzionalità aggiuntive, la classe PizzaGuy deve sempre conoscere l'implementazione di PizzaOrderer. È un'idea migliore per l'oggetto PizzaGuy chiedere semplicemente a PizzaOrderer dei soldi. Il ragazzo pizza può quindi essere riutilizzato per diverse istanze della classe PizzaOrder come:

SearchesCouchForMoneyPizzaOrderer 
PayingWithDebitForATenDollarOrderGuy

...

Questa è chiamata la legge di Demeter ed è una regola importante per ridurre l'accoppiamento, consentendo il riutilizzo del codice.

    
risposta data 08.12.2014 - 16:32
fonte
1

Basically I have these checks integrated into the class right now - isCouponUsed() calls isCouponValid() in order to check that it's valid first. Is this a good idea, or is it better for me to use isCouponValid() outside of the class and then pass that value to isCouponUsed(), such that isCouponUsed() should just be concerned with validating a coupon and not check if that code is valid first?

Un metodo che chiama altri metodi per eseguire il suo lavoro è soddisfacente e si adatta bene alla progettazione orientata agli oggetti. Pensa agli oggetti come a persone che eseguono un comportamento nel sistema. Meno una singola persona deve fare il più agevole il sistema funzionerà.

Immagina di chiamare un call center per ordinare un nuovo telefono. Parli con un cliente di fronte a "oggetto", ma quel "oggetto" potrebbe interrogare un'intera schiera di altri oggetti interni ai sistemi per eseguire il comportamento di "fornire al cliente un nuovo telefono". Come cliente esterno non vedi né ti preoccupi di nulla di tutto ciò, ma il call centre potrebbe cadere in parte se il ragazzo al telefono dovesse anche controllare l'inventario, correre al magazzino a prendere il telefono, pacchettarlo, postarlo ecc.

Allo stesso modo, se un coupon deve prima verificare se è valido prima di controllare se è usato va bene. L'oggetto esterno che ha inviato il messaggio "sei valido" all'oggetto coupon non si preoccupa di ciò che fa per eseguire questa azione, ma separare la responsabilità da un numero di oggetti più piccoli con un comportamento chiaro è un buon design (nella stessa in modo che il tizio che risponde al telefono nel call center non esegua tutte le azioni necessarie per ottenere un nuovo telefono, sarebbe sciocco)

Il tuo codice potrebbe avere più senso se hai spostato i metodi nell'oggetto coupon. Perché il coupon non può convalidare se stesso e tracciare se viene utilizzato da solo. Probabilmente ridurrai notevolmente il numero di metodi pubblici sull'oggetto.

    
risposta data 09.12.2014 - 16:30
fonte

Leggi altre domande sui tag