Progettare correttamente una classe astratta: come recuperare i dati?

Ho intenzione di fare un altro suggerimento in più per ampliare la tua prospettiva. Mentre raccomando i concetti alla base di ciò che dirò in generale, avrei bisogno di più contesto per vedere se è più appropriato per la tua situazione (e probabilmente suggerirebbe modifiche ad altro codice).

Hai assolutamente colpito l'unghia sulla testa quando menzioni le tue preoccupazioni sul dover ricordare di chiamare metodi. La situazione è potenzialmente peggiore. Anche se ricordo di chiamare i metodi, cosa succede se chiamo di nuovo GetData più tardi? Mentre il codice è attualmente scritto, si finisce con un oggetto con dati incoerenti. (Per inciso, dovresti probabilmente guardare Dependency Injection [il concetto precedentemente noto come "parametrizzazione"] per spostare i dettagli come dove recuperare i dati dalle tue classi. Non mi concentrerò su questo di seguito.)

Una soluzione a questo problema è utilizzare il sistema di tipi per aiutare a rafforzare i tuoi invarianti. Ecco a cosa serve. Per rendere l'esempio un po 'più incisivo, assumerò che vi sia una necessità pressante di avere un controllo esplicito su quando un report viene analizzato. (Nel tuo codice attuale non è chiaro il motivo per cui esporre GetData . Se non lo si vuole fare nel costruttore, farlo eseguire su richiesta da ParseData .) L'idea chiave è:

Make a different type for each stage of processing.

Considera qualcosa come:

public interface IReportDataProvider {
    IUnparsedReport Fetch();
}

public interface IUnparsedReport {
    // ...
    IParsedReport Parse();
}

public interface IParsedReport {
    // ...
    string ToJsonString();
}

Con questo approccio, ora non sei in grado di fare le cose fuori ordine, o dimentica di fare un passo, o di produrre dati incoerenti duplicando un passo. L'applicazione di questa tecnica in realtà non richiede un sistema di tipi. È solo l'astrazione dei dati. Un posto in cui un sistema di tipi può essere prezioso in questo è una possibile implementazione di Parse in alcuni contesti potrebbe essere return this; . In questo scenario, stiamo utilizzando il sistema di tipi per applicare gli invarianti perché se chiedi solo un IUnparsedReport , non sai che l'istanza specifica che ottieni implementa anche IParsedReport (a meno che tu non faccia qualcosa di brutto come un downcast o riflessione). In effetti, puoi applicare questo approccio al tuo codice quasi così com'è. Questo può evitare di richiedere wrapper o copiare dati in giro o duplicare il codice. Non è consigliabile farlo di default, poiché implica la mutazione e può portare a una condivisione nascosta che genera risultati confusi.

Per lanciare questo approccio nei termini OOD, puoi visualizzare ogni fase come una Fabbrica che produce la fase successiva. Ho usato una terminologia simile a quella della risposta di BobDalgleish in quanto il DataProvider discusso può essere visto come uno stadio. (Anche se dipende da quanto sia generale, potrebbe essere meglio non pensare in questo modo e basta fornirlo come parametro per un'implementazione specifica di IUnparsedReport .) Non sono d'accordo con la descrizione della classe anche se, come riassume semplicemente il problema in DataProvider . Cosa succede se ti dimentichi di "connetterti"? Puoi utilizzare le stesse tecniche in questa risposta per risolvere i problemi di gestione temporanea della descrizione di DataProvider .

Esiste un mantra più ampio, di cui questa è un'istanza, popolare nella comunità di programmazione funzionale tipizzata, sebbene non sia affatto limitata al FP o alle comunità di programmazione tipizzate. Vale a dire:

Make invalid states unrepresentable.

Would it be better to separate GetData from the class? Then I could retrieve the data first and load it into the ParseData function as an argument

Questa è la strada da percorrere. Fornisci un'istanza di classe DataProvider al costruttore per la tua sottoclasse Report . L'interfaccia DataProvider offrirà metodi per connettere / disconnettere e interrogare i dati. Come bonus, questo ti permetterà di costruire lo scaffold del test per testare i tuoi rapporti con specifiche istanze di DataProvider che soddisfano le tue esigenze di test.

They all share the following properties which will all be the same type

Questo non dovrebbe mai essere un criterio per definire una classe astratta. Utilizzando questo approccio, ereditarietà strutturale , che interrompe l'incapsulamento , è fragile e tutto sommato procedurale.

They also all have to retrieve the data from a database (currently the same database, could change in the future) and parse the data into an object, and be able to convert that object into a JSON.

È molto meglio! Hai identificato la tua astrazione: cosa i tuoi rapporti dovrebbero fare.

interface Report
{
    public function initialize();

    public function parse();

    public function toJson();
}

Tuttavia, come hai detto, esiste un accoppiamento temporale nel modo in cui utilizzi il rapporto : %codice%. Non mi piace neanche. E sono d'accordo che i costruttori dovrebbero essere leggeri.

Dopo aver identificato questi problemi, guardo da vicino la mia astrazione. Cos'è tutto questo? Si tratta di ottenere e presentare i dati. Questo è il comportamento intrinseco di un rapporto. Quindi probabilmente potrei astrarre queste operazioni. Come hai notato, potrebbero esserci diverse fonti di dati e ho il sospetto che ci sia più della sola rappresentazione JSON. Quindi ho due gerarchie separate, che rappresentano il recupero dei dati e la rappresentazione dei dati:

interface Report
{
    public function display();
}

interface DataStorage
{
    /**
     * @return array
     */
    public function get();
}

E l'implementazione potrebbe essere come la seguente:

class ArrayedReport implements Report
{
    private $reportId;
    private $startDate;
    private $finishDate;
    private $storage;

    public function __construct(ReportId $reportId, DateTime $startDate, DateTime $finishDate, DataStorage $storage)
    {
        $this->reportId = $reportId;
        $this->startDate = $startDate;
        $this->finishDate = $finishDate;
        $this->storage = $storage;
    }

    public function display()
    {
        var_export(
            $this->storage->get(
                $this->reportId,
                $this->startDate,
                $this->reportId
            )
        );
    }
}

class JsonReport implements Report
{
    private $reportId;
    private $startDate;
    private $finishDate;
    private $storage;

    public function __construct(ReportId $reportId, DateTime $startDate, DateTime $finishDate, DataStorage $storage)
    {
        $this->reportId = $reportId;
        $this->startDate = $startDate;
        $this->finishDate = $finishDate;
        $this->storage = $storage;
    }

    public function display()
    {
        echo
            json_encode(
                $this->storage->get(
                    $this->reportId,
                    $this->startDate,
                    $this->reportId
                )
            );
    }
}

Ora noto che anche i criteri su cui vengono recuperati i dati possono essere astratti. Questo può portarmi a una terza astrazione:

interface Criteria
{
    /**
     * @return array
     */
    public function value();
}

interface DataStorage
{
    /**
     * @param Criteria $criteria
     * @return array
     */
    public function get(Criteria $criteria);
}

interface ReportRepresentation
{
    public function display();
}

Questo può essere implementato come segue:

class JsonReport implements ReportRepresentation
{
    private $criteria;
    private $storage;

    public function __construct(Criteria $criteria, DataStorage $storage)
    {
        $this->criteria = $criteria;
        $this->storage = $storage;
    }

    public function display()
    {
        echo json_encode($this->storage->get($this->criteria));
    }
}

Nota che non ho deliberatamente introdotto un'astrazione chiamata I have to remember to call GetData() and ParseData() . Sarebbe semplicemente una struttura di dati per mantenere un criterio e l'archiviazione dei dati, e sicuramente avrebbe una singola implementazione. Quindi, sarebbe un'astrazione inutile.

E ora, ho tre astrazioni con più implementazioni, che possono essere composte in una miriade di modi per scopi diversi, incluso un facile test unitario. Questo è il mio criterio di buona decomposizione del dominio , quello che rende il design veramente solido .