Unità testare metodi protetti che non hanno motivo di essere pubblici

0

Ho bisogno di creare un sistema di sponsorizzazione con requisiti aziendali complessi. Fondamentalmente, dopo che un utente effettua un pagamento, il sistema dovrebbe essere attivato. Esistono molti tipi diversi di sponsorizzazione, quindi ho trovato astrazioni per rendere la mia vita più semplice ora e in futuro.

Per usare il polimorfismo, ho creato per prima cosa una classe astratta che erediterebbe ogni sistema sponsor e implementare ogni metodo astratto necessario.

Ecco la classe:

abstract class SponsorshipService
{
    /** @var UserKang */
    protected $user;
    /** @var UserKang */
    protected $sponsor;
    /** @var SponsorUser */
    protected $sponsorship;
    /** @var int */
    protected $amount;
    /** @var BillingMicroService */
    protected $billingMicroService;
    /** @var MailMicroService */
    protected $mailMicroService;

    public function __construct(BillingMicroService $billingMicroService, MailMicroService $mailMicroService)
    {
        $this->billingMicroService = $billingMicroService;
        $this->mailMicroService = $mailMicroService;
    }

    public function apply(UserKang $user, int $amount): bool
    {
        $this->user = $user;
        $this->amount = $amount;

        if (!$this->user->isSponsored()) {
            return false;
        }

        $this->sponsor = $this->user->getSponsor();
        $this->sponsorship = $this->user->getSponsorship();

        if (!$this->checkSponsorshipAppliance()) {
            return false;
        }

        $offeredCash = $this->getOfferedCash();
        $isCreditOffered = $this->offerCreditSponsorship($offeredCash);

        if (!$isCreditOffered) {
            return false;
        }

        $this->sendEmail();
        $this->updateSponsorship();
    }

    abstract protected function checkSponsorshipAppliance(): bool;

    abstract protected function getOfferedCash(): int;

    abstract protected function offerCreditSponsorship(): bool;

    abstract protected function sendEmail(): void;

    abstract protected function updateSponsorship(): void;
}

Questo è tutto buono finora, ma mi piacerebbe testare ciascuno di questi metodi astratti per ogni implementazione di SponsorshipService che farò. Ma li ho resi protetti, perché non sembra che abbiano alcuna ragione per essere pubblici, e penso che i metodi meno pubblici di una classe lo siano, più è facile da usare e anche più sicuro.

Cosa c'è che non va in questo design? Chiaramente non voglio testare unitamente apply in ogni modo possibile per ogni implementazione, dato che sarebbe un incubo tanto quanto lo sarebbe in futuro.

    
posta Steve Chamaillard 09.12.2018 - 14:32
fonte

1 risposta

6

What's wrong in this design?

È una domanda soggettiva, anche se ecco alcuni spunti di riflessione:

Il costruttore inizializza due membri di istanza ( billingMicroService , mailMicroService ), ma non tutti i membri di istanza - gli altri vengono inizializzati in apply .

Quando abbiamo campi di istanze che possono essere raggruppati in due (o più) serie distinte di vite all'interno di una classe, suggerisce di aver confuso quelli che potrebbero essere concetti / entità di dominio indipendenti - e questo suggerisce di rifare in due (o più ) classi. (A volte una serie di campi di istanza potrebbe diventare parametri anziché un'altra classe.)

apply inizializza anche condizionalmente alcuni campi che suggeriscono una possibile ulteriore conflazione.

Ti aspetti una singola sottoclasse concreta per implementare tutte le funzionalità che stai offrendo nella base astratta, come l'invio di e-mail e le offerte di denaro e le offerte di credito?

Sembra una violazione di una singola responsabilità, per esempio.

Ma per un altro, se vuoi combinare le abilità per inviare e-mail, con diversi modi per offrire denaro, con diversi modi di offrire credito, dovrai creare una nuova sottoclasse per ogni combinazione - questo si traduce in esplosione di classe, e può essere affrontata per composizione.

(Anche se è vero che puoi introdurre sottoclassi astratte che offrono funzionalità parziali in modo da avere più DRY (riutilizzo) della re-implementazione di elementi in ogni sottoclasse concreta, gestendo una gerarchia di classi per selezionare le funzionalità in questo modo è difficile non necessario poiché questo può essere fatto più semplicemente con la composizione.)

Se utilizzi la composizione invece dell'ereditarietà, credo che troverai più facile testare le singole funzionalità.

    
risposta data 09.12.2018 - 16:25
fonte

Leggi altre domande sui tag