Tipi di dati a responsabilità singola e personalizzati

10

Negli ultimi mesi ho chiesto a persone qui a SE e su altri siti di offrirmi alcune critiche costruttive riguardo al mio codice. C'è una cosa che è saltata fuori quasi ogni volta e ancora non sono d'accordo con quella raccomandazione; : P Mi piacerebbe discuterne qui e forse le cose diventeranno più chiare per me.

Riguarda il principio di responsabilità singola (SRP). Fondamentalmente, ho una classe dati, Font , che non solo contiene funzioni per manipolare i dati, ma anche per caricarli. Mi hanno 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 dalla mia classe di caratteri

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 consigliato

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. La classe Font non è più autosufficiente (è inutile senza la factory) e FontFactory ha bisogno di conoscere i dettagli sull'implementazione della risorsa, che è probabilmente fatta attraverso l'amicizia o 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: essendo autosufficiente, è più facile da capire e mantenere. Inoltre, puoi usare la classe senza dover aggiungere altro. Se, tuttavia, trovi che hai bisogno di una gestione delle risorse più complessa (una fabbrica) puoi farlo facilmente (in seguito parlerò della mia fabbrica, ResourceManager<Font> ).

  • Segue la libreria standard - credo che i tipi definiti dall'utente dovrebbero cercare il più possibile di copiare il comportamento dei tipi standard in quella lingua. Il std::fstream è autosufficiente e fornisce funzioni come open e close . Seguire la libreria standard significa che non è necessario spendere sforzi per imparare un altro modo di fare le cose. Inoltre, in generale, il comitato standard C ++ probabilmente conosce più sul design di chiunque altro qui, quindi, in caso di dubbio, copia quello che fanno.

  • Testability - Qualcosa non funziona, dove potrebbe essere il problema? - È il modo in cui Font gestisce i suoi dati o il modo in cui FontFactory ha caricato i dati? Non lo sai davvero. Avere le classi autosufficienti riduce questo problema: puoi testare Font in isolamento. Se poi devi testare la fabbrica e sai che Font funziona bene, saprai anche che ogni volta che si verifica un problema, deve essere all'interno della fabbrica.

  • È agnostico di contesto - (Questo interseca un po 'il mio primo punto.) Font fa la sua cosa e non fa ipotesi su come lo userai: puoi usarlo come più ti piace. Costringere l'utente ad usare una fabbrica aumenta l'accoppiamento tra le classi.

Anch'io ho una fabbrica

(Perché il design di Font mi consente di.)

O piuttosto più di un manager, non solo una fabbrica ... Font è autosufficiente quindi il gestore non ha bisogno di sapere come costruirne uno; invece il manager si assicura che lo stesso file o buffer non venga caricato in memoria più di una volta. Si potrebbe dire che una fabbrica può fare lo stesso, ma non romperebbe l'SRP? La fabbrica quindi non solo deve 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 utilizzato il gestore. Si noti che è usato fondamentalmente 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...
}

Bottom Line ...

(Vorresti mettere un tl; dr qui, ma non riesco a pensarne uno.: \)
Bene, ce l'hai, ho fatto il mio caso nel miglior modo possibile. Si prega di postare eventuali contro-argomenti che avete e anche eventuali vantaggi che pensate che il design suggerito abbia sul mio stesso design. Fondamentalmente, prova a mostrarmi che mi sbaglio. :)

    
posta Paul 06.06.2011 - 11:15
fonte

2 risposte

7

A mio parere non c'è niente di sbagliato in questo codice, fa ciò che ti serve in modo sensato e ragionevolmente facile da mantenere.

Tuttavia , il problema che si ha con questo codice è che se si vuole fare qualsiasi altra cosa si dovrà cambiare tutto .

Il punto dell'SRP è che se hai un singolo componente 'CompA' che fa l'algoritmo A () e hai bisogno di cambiare l'algoritmo A () non dovresti cambiare "CompB".

Le mie capacità in C ++ sono troppo arrugginite per suggerire uno scenario decente in cui dovrai modificare la soluzione di gestione dei font, ma il solito caso che faccio è l'idea di scorrere in un livello di cache. Idealmente non vuoi la cosa che carica cose da sapere da dove proviene, né la cosa che viene caricata importa da dove viene, perché fare modifiche è più semplice. Si tratta di manutenibilità.

Un esempio potrebbe essere il caricamento del font da una terza fonte (ad esempio un'immagine di sprite del personaggio). Per raggiungere questo obiettivo è necessario cambiare il caricatore (per chiamare il terzo metodo se i primi due falliscono) e la classe Font stessa per implementare questa terza chiamata. Idealmente, dovresti semplicemente creare un altro factory (SpriteFontFactory, o qualsiasi altra cosa), implementare lo stesso metodo loadFont (...) e incollarlo in un elenco di fabbriche da qualche parte che può essere usato per caricare il font.

    
risposta data 06.06.2011 - 12:11
fonte
0

Una cosa che mi infastidisce della tua classe è che hai metodi loadFromMemory e loadFromFile . Idealmente, dovresti avere solo il metodo loadFromMemory ; un font non dovrebbe preoccuparsi di come i dati in memoria sono diventati. Un'altra cosa è che dovresti usare il costruttore / distruttore invece dei metodi load e free . Pertanto, loadFromMemory diventerebbe Font(const void *buf, int len) e free() diventerebbe ~Font() .

    
risposta data 06.06.2011 - 15:05
fonte

Leggi altre domande sui tag