Responsabilidad individual y tipos de datos personalizados

10

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 Fontclase ya no es autosuficiente (es inútil sin la fábrica), y FontFactorynecesita 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:

  • Fontes 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ábrica ResourceManager<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::fstreames autosuficiente y proporciona funciones como openy close. 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 Fontmaneja sus datos o la forma en que los FontFactorycarga? Realmente no lo sabes. Hacer que las clases sean autosuficientes reduce este problema: puede realizar pruebas Fontde forma aislada. Si luego tiene que probar la fábrica y sabe que Fontfunciona 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) Fonthace 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 Fontme lo permite).

O más bien un gerente, no simplemente una fábrica ... Fontes 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. :)

Paul
fuente
2
Me recuerda a ActiveRecord de Martin Fowler vs DataMapper .
Usuario
Proporcione conveniencia (su diseño actual) en la interfaz externa más orientada al usuario. Use SRP internamente para facilitar sus futuros cambios de implementación. Puedo pensar en adornos del cargador de fuentes que omiten la cursiva y la negrita; que solo carga el BMP Unicode, etc.
rwong
@rwong Sé de esa presentación, tenía un marcador ( video ). :) Pero no entiendo lo que dices en tu otro comentario ...
Paul
1
@rwong ¿No es ya un trazador de líneas? Solo necesita una línea, ya sea que cargue una fuente directamente o mediante ResourceManager. ¿Y qué me impide volver a implementar el RM si los usuarios se quejan?
Paul

Respuestas:

7

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.

Ed James
fuente
1
Ah, ya veo: si agrego una forma más de cargar una fuente, necesitaré agregar una función de adquisición más al administrador y una función de carga más al recurso. De hecho, esa es una desventaja. Sin embargo, dependiendo de cuál sea esta nueva fuente, es probable que tenga que manejar los datos de manera diferente (los TTF son una cosa, los sprites de fuente son otra), por lo que no puede predecir cuán flexible será un determinado diseño. Sin embargo, sí entiendo tu punto.
Paul
Sí, como dije, mis habilidades en C ++ están bastante oxidadas, así que luché para encontrar una demostración viable del problema, estoy de acuerdo con lo de la flexibilidad. Realmente depende de lo que esté buscando con su código, como dije, creo que su código original es una solución razonablemente razonable al problema.
Ed James
Gran pregunta y gran respuesta, y lo mejor es que múltiples desarrolladores pueden aprender de ella. Por eso me encanta pasar el rato aquí :). Ah, y mi comentario no es completamente redundante, SRP puede ser un poco complicado porque tienes que preguntarte 'qué pasaría si', lo que puede parecer contrario a: 'la optimización prematura es la raíz de todo mal' o ' Las filosofías de YAGNI. ¡Nunca hay una respuesta en blanco y negro!
Martijn Verburg
0

Una cosa que me molesta acerca de su clase es que usted tiene loadFromMemoryy loadFromFilemétodos. Idealmente, debería tener un único loadFromMemorymé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 y freemétodos. Por loadFromMemorylo tanto, se convertiría Font(const void *buf, int len)y free()se volvería ~Font().

zvrba
fuente
Se puede acceder a las funciones de carga desde dos constructores, y free se llama en el destructor. Simplemente no lo mostré aquí. Me parece conveniente poder cargar la fuente directamente desde un archivo, en lugar de abrir primero el archivo, escribir los datos en un búfer y luego pasarlos a la Fuente. Sin embargo, a veces también necesito cargar desde un búfer, por eso tengo ambos métodos.
Paul