En los últimos meses, he pedido que personas aquí en SE y en otros sitios me ofrezcan algunas críticas constructivas con respecto a mi código. Hay una cosa que sigue apareciendo casi siempre y todavía no estoy de acuerdo con esa recomendación; : P Me gustaría discutirlo aquí y tal vez las cosas se aclaren para mí.
Se trata del principio de responsabilidad única (SRP). Básicamente, tengo una clase de datos Font
, que no solo tiene funciones para manipular los datos, sino también para cargarlos. Me dijeron que los dos deberían estar separados, que las funciones de carga deberían ubicarse dentro de una clase de fábrica; Creo que esta es una mala interpretación del SRP ...
Un fragmento de mi clase de fuente
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();
};
Diseño Sugerido
class Font
{
public:
void some();
void another();
};
class FontFactory
{
public:
virtual std::unique_ptr<Font> createFromFile(...) = 0;
virtual std::unique_ptr<Font> createFromMemory(...) = 0;
};
El diseño sugerido supuestamente sigue el SRP, pero no estoy de acuerdo, creo que va demasiado lejos. La Font
clase ya no es autosuficiente (es inútil sin la fábrica), y FontFactory
necesita conocer detalles sobre la implementación del recurso, que probablemente se realiza a través de la amistad o el público, lo que expone aún más la implementación de Font
. Creo que esto es más bien un caso de responsabilidad fragmentada .
He aquí por qué creo que mi enfoque es mejor:
Font
es autosuficiente: siendo autosuficiente, es más fácil de entender y mantener. Además, puede usar la clase sin tener que incluir nada más. Sin embargo, si encuentra que necesita una administración de recursos más compleja (una fábrica), también puede hacerlo fácilmente (más adelante hablaré de mi propia fábricaResourceManager<Font>
).Sigue la biblioteca estándar: creo que los tipos definidos por el usuario deberían intentar todo lo posible para copiar el comportamiento de los tipos estándar en ese idioma respectivo. El
std::fstream
es autosuficiente y proporciona funciones comoopen
yclose
. Seguir la biblioteca estándar significa que no hay necesidad de gastar esfuerzo en aprender otra forma de hacer las cosas. Además, en términos generales, el comité estándar de C ++ probablemente sepa más sobre el diseño que nadie aquí, así que si alguna vez tiene dudas, copie lo que hacen.Testabilidad: algo sale mal, ¿dónde podría estar el problema? - ¿Es la forma en que
Font
maneja sus datos o la forma en que losFontFactory
carga? Realmente no lo sabes. Hacer que las clases sean autosuficientes reduce este problema: puede realizar pruebasFont
de forma aislada. Si luego tiene que probar la fábrica y sabe queFont
funciona bien, también sabrá que cada vez que se produce un problema debe estar dentro de la fábrica.Es independiente del contexto: (Esto se cruza un poco con mi primer punto)
Font
hace lo suyo y no hace suposiciones sobre cómo lo usará: puede usarlo de la manera que desee. Obligar al usuario a usar una fábrica aumenta el acoplamiento entre clases.
Yo también tengo una fábrica
(Porque el diseño de Font
me lo permite).
O más bien un gerente, no simplemente una fábrica ... Font
es autosuficiente, por lo que el gerente no necesita saber cómo construir uno; en cambio, el administrador se asegura de que el mismo archivo o búfer no se cargue en la memoria más de una vez. Se podría decir que una fábrica puede hacer lo mismo, pero ¿eso no rompería el SRP? La fábrica no solo tendría que construir objetos, sino también administrarlos.
template<class T>
class ResourceManager
{
public:
ResourcePtr<T> acquire(const std::string& file);
ResourcePtr<T> acquire(const void* buffer, std::size_t size);
};
Aquí hay una demostración de cómo se podría usar el gerente. Tenga en cuenta que se usa básicamente exactamente como lo haría una fábrica.
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...
}
Línea de fondo...
(Me gustaría poner un tl; dr aquí, pero no puedo pensar en uno.: \)
Bueno, ahí lo tienes, he presentado mi caso lo mejor que pude. Publique cualquier contraargumento que tenga y también cualquier ventaja que crea que el diseño sugerido tiene sobre mi propio diseño. Básicamente, intenta mostrarme que estoy equivocado. :)
Respuestas:
En mi opinión, no hay nada de malo en ese código, hace lo que necesita de una manera sensata y razonablemente fácil de mantener.
Sin embargo , el problema que tiene con este código es que si quiere que haga algo más , tendrá que cambiarlo todo .
El punto de SRP es que si tiene un solo componente 'CompA' que hace el algoritmo A () y necesita cambiar el algoritmo A (), no debería tener que cambiar también 'CompB'.
Mis habilidades en C ++ están demasiado oxidadas para sugerir un escenario decente en el que necesitarás cambiar tu solución de administración de fuentes, pero el caso habitual que planteo es la idea de deslizarme en una capa de almacenamiento en caché. Idealmente, usted no quiere que la cosa que carga cosas sepa de dónde viene, ni la cosa que se está cargando debe importar de dónde viene, porque hacer cambios es más simple. Se trata de mantenibilidad.
Un ejemplo podría ser que está cargando su fuente desde una tercera fuente (digamos una imagen de sprite de caracteres). Para lograr esto, deberá cambiar su cargador (para llamar al tercer método si fallan los dos primeros) y la clase Font en sí para implementar esta tercera llamada. Lo ideal sería crear otra fábrica (SpriteFontFactory, o lo que sea), implementar el mismo método loadFont (...) y pegarlo en una lista de fábricas en algún lugar que pueda usarse para cargar la fuente.
fuente
Una cosa que me molesta acerca de su clase es que usted tiene
loadFromMemory
yloadFromFile
métodos. Idealmente, debería tener un únicoloadFromMemory
método; una fuente no debería importar cómo llegaron a ser los datos en la memoria. Otra cosa es que debe usar constructor / destructor en lugar de carga yfree
métodos. PorloadFromMemory
lo tanto, se convertiríaFont(const void *buf, int len)
yfree()
se volvería~Font()
.fuente