Typy pojedynczej odpowiedzialności i niestandardowe typy danych

W ciągu ostatnich miesięcy prosiłem o ludzi tutaj w SE i na innych stronach oferujących mi konstruktywną krytykę mojego kodu. Jest jedna rzecz, która pojawia się prawie za każdym razem, a ja nadal nie zgadzam się z tym zaleceniem; : P Chciałbym to przedyskutować tutaj i być może wszystko stanie się dla mnie bardziej zrozumiałe.

Dotyczy to zasady pojedynczej odpowiedzialności (SRP). Zasadniczo mam klasę danych Font, która zawiera nie tylko funkcje do manipulowania danymi, ale także do ich ładowania. Powiedziano mi, że oba powinny być oddzielne, że funkcje ładujące powinny być umieszczone w klasie fabrycznej; Myślę, że to błędna interpretacja SRP ...

Fragment z My Class 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();
};

Sugerowany projekt

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


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

Sugerowany projekt prawdopodobnie jest zgodny z SRP, ale nie zgadzam się - myślę, że posuwa się za daleko. FontKlasa nie jest już samowystarczalny (jest bezużyteczny bez fabrycznie) i FontFactorymusi znać szczegóły dotyczące realizacji zasobu, który jest prawdopodobnie wykonanej przez przyjaźni lub pobierające publicznych, co dodatkowo naraża realizację Font. Myślę, że to raczej fragmentaryczna odpowiedzialność .

Oto dlaczego myślę, że moje podejście jest lepsze:

• Fontjest samowystarczalny - Będąc samowystarczalnym, łatwiej jest go zrozumieć i utrzymać. Możesz także użyć tej klasy bez konieczności dołączania czegokolwiek innego. Jeśli jednak okaże się, że potrzebujesz bardziej złożonego zarządzania zasobami (fabryką), możesz to łatwo zrobić (później opowiem o mojej własnej fabryce ResourceManager<Font>).

• Podąża za standardową biblioteką - uważam, że typy zdefiniowane przez użytkownika powinny jak najwięcej próbować skopiować zachowanie standardowych typów w tym języku. std::fstreamJest samowystarczalny i zapewnia funkcje, jak openi close. Postępowanie zgodnie ze standardową biblioteką oznacza, że ​​nie trzeba poświęcać wysiłku na naukę jeszcze jednego sposobu działania. Poza tym, ogólnie mówiąc, komitet standardowy C ++ prawdopodobnie wie więcej o projektowaniu niż ktokolwiek tutaj, więc jeśli masz wątpliwości, skopiuj to, co robią.

• Testowalność - coś poszło nie tak, gdzie może być problem? - Czy to sposób, w jaki Fontobsługuje dane, czy sposób, w jaki FontFactorydane zostały załadowane? Naprawdę nie wiesz. Samowystarczalność klas zmniejsza ten problem: możesz testować Fontw izolacji. Jeśli następnie musisz przetestować fabrykę i wiesz, że Fontdziała dobrze, będziesz wiedział, że ilekroć wystąpi problem, musi on znajdować się wewnątrz fabryki.

• Jest Fontniezależny od kontekstu - (to trochę przecina mój pierwszy punkt.) Robi swoje i nie przyjmuje żadnych założeń, jak go używać: możesz go używać w dowolny sposób. Zmuszenie użytkownika do korzystania z fabryki zwiększa sprzężenie między klasami.

Ja też mam fabrykę

(Ponieważ konstrukcja Fontpozwala mi.)

A raczej menedżer, a nie tylko fabryka ... Fontjest samowystarczalna, więc kierownik nie musi wiedzieć, jak ją zbudować; zamiast tego menedżer upewnia się, że ten sam plik lub bufor nie jest ładowany do pamięci więcej niż raz. Można powiedzieć, że fabryka może zrobić to samo, ale czy to nie złamie SRP? Fabryka musiałaby wówczas nie tylko budować obiekty, ale także nimi zarządzać.

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

Oto przykład wykorzystania menedżera. Zauważ, że jest używany właściwie dokładnie tak jak fabryka.

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

Dolna linia ...

(Chciałbym umieścić tutaj tl; dr, ale nie mogę wymyślić żadnego.: \)
Cóż, proszę bardzo, przedstawiłem moją sprawę najlepiej jak potrafiłem. Proszę zamieścić wszelkie kontrargumenty, które posiadasz, a także wszelkie zalety, które Twoim zdaniem sugerowany projekt ma w stosunku do mojego własnego. Zasadniczo spróbuj mi pokazać, że się mylę. :)

Moim zdaniem nie ma nic złego w tym kodzie, robi to, czego potrzebujesz w rozsądny i dość łatwy w utrzymaniu sposób.

Jednak problem, że masz z tym kodem jest, że jeśli chcesz to zrobić cokolwiek innego będziesz musiał to wszystko zmienić .

Chodzi o to, że jeśli masz jeden komponent „CompA”, który wykonuje algorytm A () i musisz zmienić algorytm A (), nie powinieneś również zmieniać „CompB”.

Moje umiejętności w C ++ są zbyt zardzewiałe, aby sugerować dobry scenariusz, w którym będziesz musiał zmienić swoje rozwiązanie do zarządzania czcionkami, ale moim typowym przypadkiem jest przesuwanie się w warstwie buforowania. Idealnie, nie chcesz, aby rzecz, która ładuje rzeczy, wiedziała, skąd pochodzi, ani nie powinna dbać o to, skąd pochodzi, ponieważ wtedy wprowadzanie zmian jest prostsze. Chodzi o łatwość utrzymania.

Jednym z przykładów może być ładowanie czcionki z trzeciego źródła (powiedzmy obraz duszka postaci). Aby to osiągnąć, musisz zmienić moduł ładujący (aby wywołać trzecią metodę, jeśli pierwsze dwa zawiodą) i klasę Font, aby zaimplementować to trzecie wywołanie. Idealnie byłoby po prostu stworzyć inną fabrykę (SpriteFontFactory lub cokolwiek innego), wdrożyć tę samą metodę loadFont (...) i umieścić ją na liście fabryk gdzieś, gdzie można załadować czcionkę.

Jedną z rzeczy, które mnie denerwują w twojej klasie, jest to, że masz loadFromMemoryi loadFromFilemetody. Najlepiej byłoby mieć tylko loadFromMemorymetodę; czcionka nie powinna obchodzić, jak powstały dane w pamięci. Inną rzeczą jest to, że powinieneś używać konstruktora / destruktora zamiast load i freemetod. W ten sposób loadFromMemorystałby się Font(const void *buf, int len)i free()stałby się ~Font().