Einzelverantwortung und benutzerdefinierte Datentypen

In den letzten Monaten habe ich hier auf SE und auf anderen Websites nach Leuten gefragt, die mir konstruktive Kritik an meinem Code geboten haben. Es gibt eine Sache, die fast jedes Mal auftauchte, und ich stimme dieser Empfehlung immer noch nicht zu. : P Ich möchte hier darüber diskutieren und vielleicht werden mir die Dinge klarer.

Es geht um das Single-Responsibility-Prinzip (SRP). Grundsätzlich habe ich eine Datenklasse Font, die nicht nur Funktionen zum Bearbeiten, sondern auch zum Laden der Daten enthält. Mir wurde gesagt, dass die beiden getrennt sein sollten, dass Ladefunktionen innerhalb einer Fabrikklasse platziert werden sollten; Ich denke, dies ist eine Fehlinterpretation der SRP ...

Ein Fragment aus meiner Schriftklasse

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

Vorgeschlagenes Design

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


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

Das vorgeschlagene Design folgt angeblich der SRP, aber ich bin anderer Meinung - ich denke, es geht zu weit. Die FontKlasse ist nicht mehr autark (sie ist ohne die Fabrik nutzlos) und FontFactorymuss Details über die Implementierung der Ressource kennen, was wahrscheinlich durch Freundschaft oder öffentliche Getter erfolgt, die die Implementierung von weiter offenlegen Font. Ich denke, dies ist eher ein Fall fragmentierter Verantwortung .

Deshalb denke ich, dass mein Ansatz besser ist:

• Fontist autark - Autark zu sein, ist einfacher zu verstehen und zu pflegen. Sie können die Klasse auch verwenden, ohne etwas anderes einfügen zu müssen. Wenn Sie jedoch feststellen, dass Sie ein komplexeres Ressourcenmanagement benötigen (eine Fabrik), können Sie dies auch problemlos tun (später werde ich über meine eigene Fabrik sprechen ResourceManager<Font>).

• Folgt der Standardbibliothek - Ich glaube, benutzerdefinierte Typen sollten so viel wie möglich versuchen, das Verhalten der Standardtypen in der jeweiligen Sprache zu kopieren. Das std::fstreamist autark und bietet Funktionen wie openund close. Das Befolgen der Standardbibliothek bedeutet, dass Sie sich keine Mühe geben müssen, um eine weitere Methode zu erlernen. Außerdem weiß das C ++ - Standardkomitee im Allgemeinen wahrscheinlich mehr über Design als jeder andere hier. Wenn Sie also jemals Zweifel haben, kopieren Sie, was sie tun.

• Testbarkeit - Irgendwas läuft schief, wo könnte das Problem liegen? - Ist es die Art und FontWeise FontFactory, wie mit den Daten umgegangen wird, oder wie die Daten geladen werden? Du weißt es nicht wirklich. Wenn die Klassen autark sind, wird dieses Problem verringert: Sie können Fontisoliert testen . Wenn Sie dann die Fabrik zu testen und Sie wissen Fontgut funktioniert, werden Sie auch wissen , dass , wenn ein Problem auftritt es in der Fabrik sein muss.

• Es ist kontextunabhängig - (Dies überschneidet sich ein wenig mit meinem ersten Punkt.) FontMacht seine Sache und macht keine Annahmen darüber, wie Sie es verwenden werden: Sie können es verwenden, wie Sie möchten. Wenn der Benutzer gezwungen wird, eine Factory zu verwenden, wird die Kopplung zwischen Klassen erhöht.

Ich habe auch eine Fabrik

(Weil das Design von Fontmir erlaubt.)

Oder eher ein Manager, nicht nur eine Fabrik ... Fontist autark, so dass der Manager nicht wissen muss, wie man eine baut; Stattdessen stellt der Manager sicher, dass dieselbe Datei oder derselbe Puffer nicht mehr als einmal in den Speicher geladen wird. Man könnte sagen, eine Fabrik kann das Gleiche tun, aber würde das nicht die SRP brechen? Die Fabrik müsste dann Objekte nicht nur bauen, sondern auch verwalten.

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

Hier ist eine Demonstration, wie der Manager verwendet werden kann. Beachten Sie, dass es im Grunde genau wie eine Fabrik verwendet wird.

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

Endeffekt...

(Es würde gerne einen tl; dr hierher bringen, aber mir fällt keiner ein .: \)
Nun, da haben Sie es, ich habe meinen Fall so gut wie möglich gemacht. Bitte posten Sie alle Gegenargumente und auch alle Vorteile, die das vorgeschlagene Design Ihrer Meinung nach gegenüber meinem eigenen Design hat. Versuchen Sie mir grundsätzlich zu zeigen, dass ich falsch liege. :) :)

Meiner Meinung nach ist an diesem Code nichts auszusetzen. Er macht das, was Sie brauchen, auf vernünftige und einigermaßen pflegeleichte Weise.

Das Problem, das Sie mit diesem Code haben, ist jedoch, dass Sie alles ändern müssen , wenn Sie möchten, dass er etwas anderes tut .

Der Punkt des SRP ist, dass wenn Sie eine einzelne Komponente 'CompA' haben, die Algorithmus A () ausführt, und Sie Algorithmus A () ändern müssen, Sie auch 'CompB' nicht ändern müssen.

Meine C ++ - Kenntnisse sind zu verrostet, um ein anständiges Szenario vorzuschlagen, in dem Sie Ihre Schriftverwaltungslösung ändern müssen. Der übliche Fall ist jedoch die Idee, in eine Caching-Ebene zu gleiten. Im Idealfall möchten Sie nicht, dass das, was geladen wird, weiß, woher es kommt, und es sollte sich auch nicht darum kümmern, woher es kommt, da es dann einfacher ist, Änderungen vorzunehmen. Es geht nur um Wartbarkeit.

Ein Beispiel könnte sein, dass Sie Ihre Schriftart aus einer dritten Quelle laden (z. B. ein Zeichensprite-Bild). Um dies zu erreichen, müssen Sie Ihren Loader (um die dritte Methode aufzurufen, wenn die ersten beiden fehlschlagen) und die Font-Klasse selbst ändern, um diesen dritten Aufruf zu implementieren. Im Idealfall erstellen Sie einfach eine andere Factory (SpriteFontFactory oder was auch immer), implementieren dieselbe loadFont (...) -Methode und fügen sie in eine Liste von Fabriken ein, die zum Laden der Schriftart verwendet werden können.

Eine Sache , die nervt mich über Ihre Klasse ist , dass Sie loadFromMemoryund loadFromFileMethoden. Idealerweise sollten Sie nur eine loadFromMemoryMethode haben. Eine Schriftart sollte sich nicht darum kümmern, wie die Daten im Speicher entstanden sind. Eine andere Sache ist, dass Sie Konstruktor / Destruktor anstelle von Laden und freeMethoden verwenden sollten. So loadFromMemorywürde Font(const void *buf, int len)und free()würde werden ~Font().