Tipos de dados personalizados e de responsabilidade única

10

Nos últimos meses, pedi para as pessoas aqui no SE e em outros sites me oferecerem críticas construtivas em relação ao meu código. Há uma coisa que ficava aparecendo quase sempre e eu ainda não concordo com essa recomendação; : P Gostaria de discutir aqui e talvez as coisas se tornem mais claras para mim.

É sobre o princípio de responsabilidade única (SRP). Basicamente, eu tenho uma classe de dados Font, que não apenas contém funções para manipular os dados, mas também para carregá-los. Disseram-me que os dois deveriam estar separados, que as funções de carregamento deveriam ser colocadas dentro de uma classe de fábrica; Eu acho que isso é uma interpretação errada do SRP ...

Um fragmento da classe My Font

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();
};

Projeto 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;
};

O design sugerido segue o SRP, mas eu discordo - acho que vai longe demais. A Fontclasse não é mais auto-suficiente (é inútil sem a fábrica) e FontFactoryprecisa saber detalhes sobre a implementação do recurso, o que provavelmente é feito por meio de amizades ou getters públicos, que expõem ainda mais a implementação Font. Penso que este é um caso de responsabilidade fragmentada .

Eis por que acho que minha abordagem é melhor:

  • Fonté auto-suficiente - Sendo auto-suficiente, é mais fácil entender e manter. Além disso, você pode usar a classe sem precisar incluir mais nada. Se, no entanto, você achar que precisa de um gerenciamento mais complexo de recursos (uma fábrica), também poderá fazer isso com facilidade (mais tarde falarei sobre minha própria fábrica ResourceManager<Font>).

  • Segue a biblioteca padrão - acredito que os tipos definidos pelo usuário devem tentar o máximo possível copiar o comportamento dos tipos padrão no respectivo idioma. O std::fstreamé auto-suficiente e fornece funções como opene close. Seguir a biblioteca padrão significa que não há necessidade de gastar esforços aprendendo mais uma maneira de fazer as coisas. Além disso, de um modo geral, o comitê padrão do C ++ provavelmente sabe mais sobre design do que qualquer um aqui; portanto, se houver dúvida, copie o que eles fazem.

  • Testabilidade - Algo dá errado, onde poderia estar o problema? - É a maneira como Fontlida com os dados ou como FontFactoryos dados são carregados? Você realmente não sabe. Ter as classes auto-suficientes reduz esse problema: você pode testar Fontisoladamente. Se você precisar testar a fábrica e souber que Fontfunciona bem, também saberá que sempre que ocorre um problema, ele deve estar dentro da fábrica.

  • É independente do contexto - (isso se cruza um pouco com o meu primeiro argumento.) FontFaz sua parte e não faz suposições sobre como você o usará: você pode usá-lo como quiser. Forçar o usuário a usar uma fábrica aumenta o acoplamento entre as classes.

Eu também tenho uma fábrica

(Porque o design de Fontme permite.)

Ou melhor, um gerente, não apenas uma fábrica ... Fonté auto-suficiente para que o gerente não precise saber como construir uma; em vez disso, o gerente garante que o mesmo arquivo ou buffer não seja carregado na memória mais de uma vez. Você poderia dizer que uma fábrica pode fazer o mesmo, mas isso não quebraria o SRP? A fábrica, então, não apenas teria que construir objetos, mas também gerenciá-los.

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

Aqui está uma demonstração de como o gerente pode ser usado. Observe que é usado basicamente exatamente como uma fábrica faria.

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 ...

(Gostaria de colocar um tl; dr aqui, mas não consigo pensar em um.: \)
Bem, aí está, fiz o meu caso da melhor maneira possível. Poste quaisquer contra-argumentos que você tiver e também todas as vantagens que você acha que o design sugerido possui sobre o meu próprio design. Basicamente, tente me mostrar que estou errado. :)

Paulo
fonte
2
Me lembra o ActiveRecord de Martin Fowler vs DataMapper .
Utilizador
Ofereça conveniência (seu design atual) na interface externa e voltada para o usuário. Use o SRP internamente para facilitar suas futuras alterações na implementação. Eu posso pensar nos enfeites do carregador de fontes que pulam itálico e negrito; que carrega apenas o BMP Unicode etc.
rwong 31/07
@rwong Eu sei dessa apresentação, eu tinha um marcador para ela ( vídeo ). :) Mas eu não entendo o que você está dizendo no seu outro comentário ... #
Paul
11
@rwong Não é já um forro? Você só precisa de uma linha, seja para carregar uma fonte diretamente ou através do ResourceManager. E o que me impede de reimplementar o RM se os usuários reclamarem?
Paulo

Respostas:

7

Não há nada de errado com esse código na minha opinião, ele faz o que você precisa de maneira sensata e razoavelmente fácil de manter.

No entanto , o problema que você tem com esse código é que, se você quiser fazer qualquer outra coisa, precisará alterar tudo .

O ponto do SRP é que, se você possui um único componente 'CompA' que executa o algoritmo A () e precisa alterar o algoritmo A (), não deve ser necessário alterar também 'CompB'.

Minhas habilidades em C ++ estão muito enferrujadas para sugerir um cenário decente em que você precisará alterar sua solução de gerenciamento de fontes, mas o caso usual que faço é a ideia de deslizar em uma camada de cache. Idealmente, você não deseja que o que carrega coisas saiba de onde vem, nem o que está sendo carregado se importa com a origem, porque fazer alterações é mais simples. É tudo sobre manutenção.

Um exemplo pode ser o carregamento da fonte de uma terceira fonte (por exemplo, uma imagem de sprite de caractere). Para conseguir isso, você precisará alterar seu carregador (para chamar o terceiro método se os dois primeiros falharem) e a própria classe Font para implementar essa terceira chamada. Idealmente, você faria outra fábrica (SpriteFontFactory, ou qualquer outra coisa), implementaria o mesmo método loadFont (...) e colocaria em uma lista de fábricas em algum lugar que possa ser usado para carregar a fonte.

Ed James
fonte
11
Ah, entendo: se eu adicionar mais uma maneira de carregar uma fonte, precisarei adicionar mais uma função de aquisição ao gerenciador e mais uma função de carga ao recurso. Na verdade, essa é uma desvantagem. Dependendo de qual seja essa nova fonte, no entanto, você provavelmente terá que lidar com os dados de maneira diferente (TTFs são uma coisa, sprites de fonte são outra), portanto, não é possível prever o quão flexível será um determinado design. Eu vejo o seu ponto, no entanto.
Paulo
Sim, como eu disse, minhas habilidades em C ++ são bastante enferrujadas, por isso lutei para apresentar uma demonstração viável do problema, concordo com a questão da flexibilidade. Realmente depende do que você deseja com seu código, como eu disse, acho que seu código original é perfeitamente razoável para o problema.
Ed James
Ótima pergunta e ótima resposta, e o melhor é que vários desenvolvedores podem aprender com isso. É por isso que eu adoro sair por aqui :). Ah, e meu comentário não é completamente redundante, o SRP pode ser um pouco complicado, porque você precisa se perguntar 'e se', o que pode parecer contrário a: 'otimização prematura é a raiz de todo mal' ou ' Filosofias de YAGNI. Nunca há uma resposta em preto e branco!
Martijn Verburg
0

Uma coisa que me incomoda em sua classe é que você tem loadFromMemorye loadFromFilemétodos. Idealmente, você deve ter apenas loadFromMemorymétodo; uma fonte não deve se importar com a forma como os dados na memória foram criados. Outra coisa é que você deve usar construtor / destruidor em vez de carga e freemétodos. Assim, loadFromMemoryse tornaria Font(const void *buf, int len)e free()se tornaria ~Font().

zvrba
fonte
As funções de carregamento são acessíveis a partir de dois construtores, e free é chamado no destruidor - simplesmente não mostrei isso aqui. Acho conveniente poder carregar a fonte diretamente de um arquivo, em vez de primeiro abrir o arquivo, gravar os dados em um buffer e depois passá-los para Font. Às vezes, também preciso carregar de um buffer, e é por isso que tenho os dois métodos.
Paulo