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 comeopen
eclose
. 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 cuiFontFactory
ha caricato i dati? Non lo sai davvero. Avere le classi autosufficienti riduce questo problema: puoi testareFont
in isolamento. Se poi devi testare la fabbrica e sai cheFont
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. :)