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 Font
classe não é mais auto-suficiente (é inútil sem a fábrica) e FontFactory
precisa 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ábricaResourceManager<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 comoopen
eclose
. 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
Font
lida com os dados ou comoFontFactory
os dados são carregados? Você realmente não sabe. Ter as classes auto-suficientes reduz esse problema: você pode testarFont
isoladamente. Se você precisar testar a fábrica e souber queFont
funciona 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.)
Font
Faz 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 Font
me 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. :)
fonte
Respostas:
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.
fonte
Uma coisa que me incomoda em sua classe é que você tem
loadFromMemory
eloadFromFile
métodos. Idealmente, você deve ter apenasloadFromMemory
mé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 efree
métodos. Assim,loadFromMemory
se tornariaFont(const void *buf, int len)
efree()
se tornaria~Font()
.fonte