L'astrazione del tuo codice è un uso eccessivo dei principi SOLID?

Vorrei schierarmi con il tuo nemico su questo.

• La 'creazione di una logica di database e tabella' è chiaramente una responsabilità separata dalla logica 'ottieni dati dal database'.

• Puoi immaginare che il codice di creazione diventi piuttosto complesso se consideriamo le migrazioni di database ecc. Basta righe di codice per giustificare un file separato.

• Potresti voler limitare il codice chiamante alla creazione o al recupero

In generale, il costo in termini di tempo per realizzare qualcosa di più generico come questo è difficile da giudicare. Una persona potrebbe trovarla onerosa e a lungo carica, un'altra potrebbe farlo come standard e pensarlo più velocemente.

Secondo me YAGNI non è mai una buona argomentazione. Se è corretto e fatto o veloce, allora è accademico se qualcuno pensa che potrebbe non averne bisogno.

"This factory class' responsibility is to get me the data from the db"

È come dire "userò questa casa automobilistica per guidare al lavoro". Tu non sei. Utilizzerai una macchina creata da questo produttore per guidare al lavoro.

La responsabilità dell'auto è di lasciarti guidare. La responsabilità del produttore di automobili è quella di costruire automobili.

Capisco da dove vieni. Tutti facciamo simili "errori di scelta rapida" contemporaneamente. Ma la tua ipotesi si basa sul concetto che un repository viene utilizzato sempre e solo per raccogliere tutti i dati .

Anche se potrebbe essere il oggi , potrebbe non essere il domani .

SOLID si concentra principalmente sul futuro. L'implementazione di SOLID non migliora il tuo codice (già funzionante, non SOLIDO). L'implementazione di SOLID assicura che sarà più semplice implementare modifiche future .

IMPORTANT CONDITIONS THAT WILL ALWAYS BE EXIST:

Quando prendo a cuore la tua affermazione, che nulla cambierà mai, quindi implementare SOLID è irrilevante.

Tuttavia, ogni volta che vengono fatte affermazioni come queste, l'oratore di solito non è in grado di garantire che le cose non cambieranno mai. Lo guardano dal punto di vista di oggi, non da quello di domani.
Non puoi sempre sapere cosa arriverà in futuro. Forse inizi a utilizzare un fornitore di dati diverso, forse ora hai bisogno di unire i dati di due fornitori di dati, ...

Detto questo, non sono abbastanza sicuro di quale sia lo scopo dell'aiuto.

• Perché il metodo di questo helper non fa parte del repository stesso?
• Qual è la differenza significativa tra la chiamata diretta al metodo del repository e la chiamata al metodo helper che chiama il metodo repository?

Potrebbe essere un dettaglio che hai intenzionalmente omesso dalla tua domanda per motivi di chiarezza. In entrambi i casi, la mia risposta è la seguente: recuperare i dati e creare un'istanza della classe che recupera i dati sono due cose completamente separate.

I massoni non sono muri di mattoni. I meccanici dell'automobile non sono auto. I coltivatori di grano non sono grano. Le fabbriche dei depositi non sono repository.

Se hai una fabbrica, significa che hai trovato la necessità di disaccoppiare l'implementazione di un oggetto dal suo contratto. Le fabbriche sono brave in questo. In questo caso, la dipendenza è su ciò che ottiene i dati.

Se ora la Factory è responsabile per ottenere i dati, qual è stato il punto da sempre? Ora la dipendenza è in fabbrica. Smette di sembrare molto simile a una fabbrica. Ha un repository statico e ora metodi statici per accedere a quel repository statico. Qualsiasi classe che utilizza questa fabbrica è ora accoppiata al modo in cui la fabbrica ottiene i dati.

L'obiettivo dovrebbe essere quello di fare in modo che il repository memorizzato nella cache sia l'unico elemento statico in questo intero sistema. In questo modo, come il Repository stesso agisce è irrilevante per chiunque lo chieda.

Puoi farlo agendo sul Repository creato dalla Fabbrica:

// Creates and/or gets a cache of the repository
Repository repository = RepositoryFactory.GetRepository();

// Get all the data from the repository
data = repository.GetAllData();

Puoi farlo implementando il Singleton Pattern :

class Repository 
{
    public static readonly Instance = new Repository();

    private Repository() {
        // Connect to the repository here.
    }
}

E poi agendo su di esso:

Repository repository = Repository.Instance;
data = repository.GetAllData();

Puoi farlo utilizzando un metodo di fabbrica :

class Repository {
    private Repository() {
       // Connect to repository here
    }

    public static Repository GetRepository() {
        if (cached) {
            return CachedRepository;
        }
        else {
            return new Repository();
        }
    }
}

E poi agendo su di esso:

Repository repository = Repository.GetRepository();
data = repository.GetAllData();

In tutti i casi possiamo iniettare la dipendenza, tuttavia vogliamo che la classe lo richieda.

NeedsTheData d = new NeedsTheData(Repository.Instance);

NeedsTheData d = new NeedsTheData(Repository.GetRepository());

E nel caso della fabbrica, la fabbrica dovrebbe essere quella che si occupa del disaccoppiamento, quindi non è necessario iniettare la classe Repository.

Sento che l'esempio di codice originale lascia molto all'immaginazione:

PRIMO ESEMPIO:

public class Repsitory<TModel>
{
    public TModel GetAll(Func<TModel, bool> where)
    {
        //Will retrieve the data based on the where clause from the db
    }
}

public class RepositoryFactory
{
    public TModel GetAll<TModel>(Func<TModel, bool> where)
    {
        return GetRepository<TModel>().GetAll(where);
    }

    private Repsitory<TModel> GetRepository<TModel>()
    {
        //Checks if the cache contains an instance of the Repository<TModel> and returns it

        //IF no instance exists will create a new instance and cache it

        //Returns the new instance
    }
}

SECOND ESEMPIO:

public class Repsitory<TModel>
{
    public TModel GetAll(Func<TModel, bool> where)
    {
        //Will retrieve the data based on the where clause
    }
}

public class RepositoryFactory
{
    public Repsitory<TModel> GetRepository<TModel>()
    {
        //Checks if the cache contains an instance of the Repository<TModel> and returns it

        //IF no instance exists will create a new instance and cache it

        //Returns the new instance
    }
}

public class RepositoryHelper
{
    public TModel GetAll<TModel>(Func<TModel, bool> where)
    {
        return RepositoryFactory.GetRepository<TModel>().GetAll(where);
    }
}