Responsabilité unique et types de données personnalisés

Au cours des derniers mois, j'ai demandé à des gens ici sur SE et sur d'autres sites de m'offrir des critiques constructives concernant mon code. Il y a une chose qui ressortait presque à chaque fois et je ne suis toujours pas d'accord avec cette recommandation; : P Je voudrais en discuter ici et peut-être que les choses deviendront plus claires pour moi.

Il s'agit du principe de responsabilité unique (PRS). Fondamentalement, j'ai une classe de données Font, qui contient non seulement des fonctions pour manipuler les données, mais aussi pour les charger. On me dit que les deux devraient être séparés, que les fonctions de chargement devraient être placées dans une classe d'usine; Je pense que c'est une mauvaise interprétation du SRP ...

Un fragment de ma classe de polices

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

Conception suggérée

class Font
{
  public:
    void some();
    void another();
};


class FontFactory
{
  public:
    virtual std::unique_ptr<Font> createFromFile(...) = 0;
    virtual std::unique_ptr<Font> createFromMemory(...) = 0;
};

La conception suggérée suit censément le SRP, mais je ne suis pas d'accord - je pense que cela va trop loin. La Fontclasse n'est plus autosuffisante (elle est inutile sans l'usine) et a FontFactorybesoin de connaître les détails de la mise en œuvre de la ressource, ce qui se fait probablement par le biais d'amitié ou de getters publics, ce qui expose davantage la mise en œuvre de Font. Je pense que c'est plutôt un cas de responsabilité fragmentée .

Voici pourquoi je pense que mon approche est meilleure:

• Fontest autosuffisant - étant autosuffisant, il est plus facile à comprendre et à maintenir. En outre, vous pouvez utiliser la classe sans avoir à inclure quoi que ce soit d'autre. Si, toutefois, vous trouvez que vous avez besoin d'une gestion plus complexe des ressources (une usine), vous pouvez facilement le faire également (plus tard, je parlerai de ma propre usine ResourceManager<Font>).

• Suit la bibliothèque standard - Je pense que les types définis par l'utilisateur devraient essayer autant que possible de copier le comportement des types standard dans cette langue respective. Le std::fstreamest autosuffisant et fournit des fonctions comme openet close. Suivre la bibliothèque standard signifie qu'il n'est pas nécessaire de consacrer des efforts à apprendre une autre façon de faire les choses. En outre, d'une manière générale, le comité standard C ++ en sait probablement plus sur la conception que quiconque ici, donc en cas de doute, copiez ce qu'ils font.

• Testabilité - Quelque chose ne va pas, où pourrait être le problème? - Est-ce la manière de Fontgérer ses données ou la façon dont FontFactoryles données ont été chargées? Tu ne sais pas vraiment. Le fait que les classes soient autonomes réduit ce problème: vous pouvez tester Fontde manière isolée. Si vous devez ensuite tester l'usine et que vous savez que cela Fontfonctionne bien, vous saurez également que chaque fois qu'un problème survient, il doit être à l'intérieur de l'usine.

• Il est indépendant du contexte - (cela recoupe un peu mon premier point.) FontFait sa chose et ne fait aucune hypothèse sur la façon dont vous allez l'utiliser: vous pouvez l'utiliser comme vous le souhaitez. Forcer l'utilisateur à utiliser une usine augmente le couplage entre les classes.

Moi aussi j'ai une usine

(Parce que la conception de Fontme le permet.)

Ou plutôt un gestionnaire, pas seulement une usine ... Fontest autosuffisant donc le gestionnaire n'a pas besoin de savoir comment en construire une; à la place, le gestionnaire s'assure que le même fichier ou tampon n'est pas chargé en mémoire plus d'une fois. On pourrait dire qu'une usine peut faire de même, mais cela ne briserait-il pas le SRP? L'usine aurait alors non seulement à construire des objets, mais aussi à les gérer.

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

Voici une démonstration de l'utilisation du gestionnaire. Notez qu'il est utilisé essentiellement exactement comme le ferait une usine.

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

Conclusion ...

(Il aimerait mettre un tl; dr ici, mais je ne peux pas penser à un.: \)
Eh bien, vous l'avez, j'ai fait mon cas du mieux que je pouvais. Veuillez poster tous les contre-arguments que vous avez et tous les avantages que vous pensez que la conception suggérée a par rapport à ma propre conception. Essentiellement, essayez de me montrer que je me trompe. :)

À mon avis, il n'y a rien de mal à ce code, il fait ce dont vous avez besoin d'une manière raisonnable et raisonnablement facile à entretenir.

Cependant , le problème que vous avez avec ce code est que si vous voulez qu'il fasse autre chose, vous devrez tout changer .

Le point du SRP est que si vous avez un seul composant «CompA» qui fait l'algorithme A () et que vous devez changer l'algorithme A (), vous ne devriez pas avoir à changer également «CompB».

Mes compétences en C ++ sont trop rouillées pour suggérer un scénario décent où vous devrez changer votre solution de gestion des polices, mais le cas habituel que je fais est l'idée de glisser dans une couche de mise en cache. Idéalement, vous ne voulez pas que la chose qui charge des trucs sache d'où elle vient, et la chose en cours de chargement ne devrait pas se soucier d'où elle vient, car il est plus simple de faire des changements. Tout est question de maintenabilité.

Un exemple pourrait être que vous chargiez votre police à partir d'une troisième source (disons une image de sprite de caractère). Pour ce faire, vous devrez changer votre chargeur (pour appeler la troisième méthode si les deux premières échouent) et la classe Font elle-même pour implémenter ce troisième appel. Idéalement, vous devez simplement créer une autre usine (SpriteFontFactory ou autre), implémenter la même méthode loadFont (...) et la coller dans une liste d'usines quelque part qui peut être utilisée pour charger la police.

Une chose qui me tracasse au sujet de votre classe est que vous avez loadFromMemoryet loadFromFileméthodes. Idéalement, vous ne devriez avoir qu'une loadFromMemoryméthode; une police ne devrait pas se soucier de la façon dont les données en mémoire sont devenues. Une autre chose est que vous devez utiliser le constructeur / destructeur au lieu de la charge et des freeméthodes. Ainsi, loadFromMemorydeviendrait Font(const void *buf, int len)et free()deviendrait ~Font().