Sto violando SRP quando inserisco il modello di fabbrica con il livello del repository?

Mentre in generale l'iniezione di qualcosa non è male e non porta automaticamente a rompere l'SRP (né lo fa nel tuo caso - hai una classe che recupera solo i dati e un'altra che costruisce un oggetto da essa), hai un problema diverso : errata comprensione della stratificazione e dell'astrazione.

Il livello del repository è quello che lega i dati ai tuoi modelli di dominio, non devi avere bisogno di un altro livello per farlo. Per non parlare della tua soluzione è troppo ingegnerizzata.

Costruisci semplicemente la citazione direttamente nel repository, a meno che tu non abbia una buona ragione per fare diversamente, senza bisogno di una fabbrica.

Dal tuo commento qui sotto:

What do you mean by binding data to domain models? It sounds too abstract and I can't find a way to understand it. Can you give an example?

Hai dati puri che in qualche modo riescono a penetrare nel tuo sistema. Le modalità possono includere:

• API SOAP,
• API REST,
• database
• contenuti da un file di lettura,
• ...

Questi dati sono solo dati e nient'altro, non contengono regole.

Hai quindi la tua attività (le parti divertenti dell'applicazione, dove sono le regole), dominio . Il problema è che il tuo dominio non comprende i dati puri. Per comprendere i dati puri, i dati devono essere trasformati per essere rappresentati da oggetti business, modelli di dominio.

Also overall you are saying, effectively merge Factory and Repository together (into Repository) but keep Quote as a separate concept, or should Quote since it contains data be bound to Repository as well?

Non sto dicendo che unirai la fabbrica e il repository insieme, sto dicendo che dovresti rimuovere completamente la factory e creare un'istanza di un oggetto Quote direttamente nel metodo getLines . E visto che ne stiamo già parlando, potrebbe essere opportuno rinominare il metodo in qualcosa di meglio, come ad esempio: getQuoteWithLineItemsById .

Inoltre, il preventivo non ha legami diretti con il deposito. Perché? Il repository funge da gateway per il tuo sistema - come ho già detto - prendendo dati puri e trasformandolo in oggetti.

Il design proposto:

class QuoteRepository
{
    /**
     * @var \Doctrine\ORM\EntityManager
     */
    private $entityManager;

    public function __construct(\Doctrine\ORM\EntityManager $entityManager)
    {
        $this->entityManager = $entityManager;
    }

    /**
     * @param string $quoteId
     * @return Quote
     */
    public function getQuoteWithLineItemsById($quoteId)
    {
        $query = $this->entityManager
            ->createQuery('SELECT s FROM... where id = :id')
            ->setParameter('id', $quoteId);

        $quote = new Quote();
        $quote->setLines($query->getResult());

        return $quote;
    }
}

class Quote
{
    private $lines = [];

    public function setLines(array $lines)
    {
        $this->lines = $lines;
    }
}

$quoteRepository = new QuoteRepository(DoctrineConnector::getEntityManager());
$quote = $quoteRepository->getQuoteWithLineItemsById('1');

Il repository ora è responsabile della trasformazione dei dati recuperati dal database in un modello di dominio, Query . Non sa nulla della logica aziendale, la logica di business deve essere all'interno del modello di dominio.

Oltre a trasformare i dati grezzi in entità comprensibili per il business, il livello del repository esiste anche per un altro motivo:

• trasformazione di entità comprensibili per il business ai dati grezzi ai fini della persistenza.

So in effect, I can end up with a single QuoteRepository class that will contain my data, have business domain functionality and read/write to/from the database?

No. Finirai con il livello del repository responsabile solo della lettura / scrittura da / verso il database e della trasformazione dei dati in un modo o nell'altro.

Quindi avrai un altro livello (il dominio) che è ignorante sulla persistenza, non sa assolutamente nulla di SQL, è puro PHP (molto probabilmente classi) e contiene tutte le tue regole di business - es. un nome utente non deve essere vuoto o più lungo di 32 caratteri.

Quando ti occupi di operazioni commerciali, i tuoi modelli di dominio assicurano che le tue regole aziendali siano preservate. Se un modello di dominio esiste e viene inviato a un repository per essere salvato, il repository non si preoccupa più dello stato del modello di dominio, perché si fida semplicemente del fatto che il modello di dominio sia in uno stato valido. La responsabilità del repository è di salvare il modello, non di controllarne lo stato.

Strettamente parlando, QuoteFactory non infrange SRP:

• mantenimento dei metodi di repository & in genere essere consapevoli di Repository non è una responsabilità, è un mezzo per la responsabilità della classe.
• creare la classe Citazione è di sua esclusiva responsabilità.

Ma QuoteFactory non sembra implementare questa responsabilità.

Secondo me, una fabbrica crea "nuove" entità, "nuove" a livello funzionale. Un'entità recuperata da un repository non è una "nuova". Un costruttore crea un oggetto, una rappresentazione tecnica di un'entità in memoria.

Quindi un metodo loadQuote() in QuoteFactory non funziona. O carica davvero una citazione "vecchia" dallo spazio di archiviazione e dovrebbe passare a QuoteRepository , oppure crea effettivamente una "nuova" citazione da alcuni elementi pubblicitari e deve essere denominata createQuote() . In entrambi i casi, QuoteFactory non ha bisogno di QuoteRepository .

Inoltre, QuoteRepository in realtà carica gli elementi pubblicitari, non le virgolette. Va bene se gli elementi pubblicitari sono principalmente parti di una citazione. Altrimenti, non dovrebbe essere chiamato LineItemRepository ?