Domanda

Negli ultimi mesi ho chiesto persone qui su SE e su altri siti mi offrono alcune critiche costruttive riguardo al mio codice. C'è una cosa che ha continuato a saltare quasi ogni volta e ancora non sono d'accordo con questa raccomandazione; : P vorrei discuterne qui e forse le cose diventeranno più chiare per me.

Si tratta del principio di responsabilità singola (SRP). Fondamentalmente, ho una classe di dati, Font, ciò non solo contiene funzioni per manipolare i dati, ma anche per il caricarli. Mi è stato detto che i due dovrebbero essere separati, che le funzioni di caricamento dovrebbero essere collocate all'interno di una classe di fabbrica; Penso che questa sia un'interpretazione errata dell'SRP ...

Un frammento della mia classe di carattere

class Font
{
  public:
    bool isLoaded() const;
    void loadFromFile(const std::string& file);
    void loadFromMemory(const void* buffer, std::size_t size);
    void free();

    void some();
    void another();
};

Design suggerito

class Font
{
  public:
    void some();
    void another();
};


class FontFactory
{
  public:
    virtual std::unique_ptr<Font> createFromFile(...) = 0;
    virtual std::unique_ptr<Font> createFromMemory(...) = 0;
};

Il design suggerito presumibilmente segue l'SRP, ma non sono d'accordo - penso che vada troppo lontano. Il Font la classe non è più autosufficiente (è inutile senza la fabbrica) e FontFactory deve conoscere i dettagli sull'attuazione della risorsa, che probabilmente viene eseguita attraverso l'amicizia o i getter pubblici, che espongono ulteriormente l'implementazione di Font. Penso che questo sia piuttosto un caso di responsabilità frammentata.

Ecco perché penso che il mio approccio sia migliore:

  • Font è autosufficiente: essere autosufficiente, è più facile da capire e mantenere. Inoltre, puoi usare la classe senza dover includere nient'altro. Se, tuttavia, scopri che hai bisogno di una gestione più complessa delle risorse (una fabbrica) puoi facilmente farlo anche (in seguito parlerò della mia fabbrica, ResourceManager<Font>).

  • Segue la libreria standard-Credo che i tipi definiti dall'utente dovrebbero provare il più possibile a copiare il comportamento dei tipi standard in quella rispettiva lingua. Il std::fstream è autosufficiente e fornisce funzioni come open e close. Seguire la biblioteca standard significa che non c'è bisogno di spendere sforzi imparando un altro modo di fare le cose. Inoltre, in generale, il comitato standard C ++ probabilmente sa di più sul design di chiunque altro qui, quindi se mai in dubbio, copia ciò che fanno.

  • Testabilità: qualcosa va storto, dove potrebbe essere il problema? - È così Font Gestisce i suoi dati o il modo FontFactory Caricato i dati? Non lo sai davvero. Avere le lezioni autosufficiente riduce questo problema: puoi testare Font in isolamento. Se allora devi testare la fabbrica e lo sai Font Funziona bene, saprai anche che ogni volta che si verifica un problema deve essere all'interno della fabbrica.

  • È agnostico del contesto - (questo si interseca un po 'con il mio primo punto.) Font fa le sue cose e non fa ipotesi su come lo userai: puoi usarlo in qualsiasi modo che preferisci. Costringere l'utente a utilizzare una fabbrica aumenta l'accoppiamento tra le classi.

Anch'io ho una fabbrica

(Perché il design di Font mi permette.)

O piuttosto più un manager, non solo una fabbrica ... Font è autosufficiente quindi il manager non ha bisogno di sapere come costruirne uno; Invece il gestore assicura che lo stesso file o buffer non venga caricato in memoria più di una volta. Potresti dire che una fabbrica può fare lo stesso, ma non si spezzerebbe l'SRP? La fabbrica non dovrebbe quindi solo costruire oggetti, ma anche gestirli.

template<class T>
class ResourceManager
{
  public:
    ResourcePtr<T> acquire(const std::string& file);
    ResourcePtr<T> acquire(const void* buffer, std::size_t size);
};

Ecco una dimostrazione di come potrebbe essere usato il manager. Si noti che viene utilizzato praticamente esattamente come farebbe una fabbrica.

void test(ResourceManager<Font>* rm)
{
    // The same file isn't loaded twice into memory.
    // I can still have as many Fonts using that file as I want, though.
    ResourcePtr<Font> font1 = rm->acquire("fonts/arial.ttf");
    ResourcePtr<Font> font2 = rm->acquire("fonts/arial.ttf");

    // Print something with the two fonts...
}

Linea di fondo...

(Vorrei mettere un tl; dr qui, ma non riesco a pensarne uno .: )
Bene, il gioco è fatto, ho reso il mio caso nel miglior modo possibile. Si prega di pubblicare eventuali contro-argomenti che hai e anche eventuali vantaggi che ritieni che il design suggerito abbia sul mio design. Fondamentalmente, prova a mostrarmi che mi sbaglio. :)

Nessuna soluzione corretta

Autorizzato sotto: CC-BY-SA insieme a attribuzione
scroll top