Индивидуальная ответственность и пользовательские типы данных

10

В последние месяцы я просил людей здесь, на SE, и на других сайтах предлагать мне конструктивную критику в отношении моего кода. Есть кое-что, что продолжало появляться почти каждый раз, и я все еще не согласен с этой рекомендацией; : P Я бы хотел обсудить это здесь, и, возможно, мне станет понятнее.

Это касается принципа единственной ответственности (SRP). По сути, у меня есть класс данных Font, который не только содержит функции для манипулирования данными, но и для их загрузки. Мне сказали, что эти два должны быть отдельными, что функции загрузки должны быть размещены внутри фабричного класса; Я думаю, что это неверное толкование SRP ...

Фрагмент из моего класса шрифтов

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

Предлагаемый дизайн

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


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

Предполагаемый дизайн предположительно следует SRP, но я не согласен - я думаю, что он заходит слишком далеко. FontКласс больше не является самодостаточным (это бесполезно без завода), и FontFactoryдолжен знать подробности о реализации ресурса, который, вероятно , сделан через дружбу или общественных добытчик, которые в дальнейшем разоблачить реализацию Font. Я думаю, что это скорее случай раздробленной ответственности .

Вот почему я думаю, что мой подход лучше:

  • Fontсамодостаточен - будучи самодостаточным, его легче понять и поддерживать. Кроме того, вы можете использовать класс без необходимости включать что-либо еще. Однако, если вы обнаружите, что вам нужно более сложное управление ресурсами (фабрика), вы можете легко это сделать (позже я расскажу о своей собственной фабрике ResourceManager<Font>).

  • Следует стандартной библиотеке - я полагаю, что пользовательские типы должны как можно больше стараться копировать поведение стандартных типов на соответствующем языке. Он std::fstreamсамодостаточен и предоставляет такие функции, как openи close. Следование стандартной библиотеке означает, что нет необходимости тратить усилия на изучение еще одного способа ведения дел. Кроме того, вообще говоря, комитет по стандартизации C ++, вероятно, знает больше о дизайне, чем кто-либо здесь, поэтому, если у вас возникнут сомнения, скопируйте то, что они делают.

  • Тестируемость - что-то идет не так, где может быть проблема? - Это способ Fontобработки своих данных или способ FontFactoryзагрузки данных? Вы действительно не знаете. Самостоятельность занятий уменьшает эту проблему: вы можете тестировать Fontизолированно. Если затем вам нужно проверить заводскую работу и вы знаете, что она Fontработает нормально, вы также будете знать, что при возникновении проблемы она должна быть на заводе.

  • Это не зависит от контекста - (это немного пересекается с моим первым пунктом.) FontДелает свое дело и не делает никаких предположений о том, как вы будете его использовать: вы можете использовать его так, как вам нравится. Принуждение пользователя к использованию фабрики увеличивает связь между классами.

У меня тоже есть фабрика

(Потому что дизайн Fontпозволяет мне.)

Или, скорее, менеджер, а не просто фабрика ... Fontсамодостаточен, поэтому менеджеру не нужно знать, как его построить; вместо этого менеджер следит за тем, чтобы один и тот же файл или буфер не загружался в память более одного раза. Можно сказать, что фабрика может делать то же самое, но разве это не нарушит SRP? Тогда фабрике придется не только строить объекты, но и управлять ими.

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

Вот демонстрация того, как менеджер может быть использован. Обратите внимание, что он используется в основном точно так же, как на фабрике.

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

Нижняя граница...

(Хотелось бы поставить здесь тролля, но я не могу вспомнить ни одного.: \)
Ну, у вас есть, я изложил свои аргументы как можно лучше. Пожалуйста, опубликуйте любые контраргументы, которые у вас есть, а также любые преимущества, которые, по вашему мнению, имеют предлагаемый дизайн по сравнению с моим собственным. По сути, попробуйте показать мне, что я не прав. :)

Павел
источник
2
Напоминает мне о ActiveRecord Мартина Фаулера против DataMapper .
Пользователь
Обеспечьте удобство (ваш текущий дизайн) в самом внешнем интерфейсе, ориентированном на пользователя. Используйте SRP для внутреннего использования, чтобы облегчить будущие изменения в реализации. Я могу думать о украшениях загрузчика шрифтов, которые пропускают курсив и полужирный; который загружает только Unicode BMP и т. д.
rwong
@ rwong Я знаю об этой презентации, у меня была закладка ( видео ). :) Но я не понимаю, что вы говорите в своем другом комментарии ...
Пол
1
@ rwong Разве это уже не один лайнер? Вам нужна только одна строка, независимо от того, загружаете ли вы шрифт напрямую или через ResourceManager. И что мешает мне повторно реализовать RM, если пользователи жалуются?
Пол

Ответы:

7

На мой взгляд, в этом коде нет ничего плохого, он делает то, что вам нужно, разумным и достаточно простым в обслуживании способом.

Однако проблема, с которой вы сталкиваетесь с этим кодом, состоит в том, что если вы хотите, чтобы он делал что-то еще, вам придется все это изменить .

Суть SRP заключается в том, что если у вас есть один компонент «CompA», который выполняет алгоритм A (), и вам нужно изменить алгоритм A (), вам также не нужно менять «CompB».

Мои навыки C ++ слишком проработаны, чтобы предложить достойный сценарий, в котором вам нужно будет изменить свое решение по управлению шрифтами, но я обычно делаю идею скользящего слоя кэширования. В идеале вы не хотите, чтобы загружающая вещь знала о том, откуда она берется, и при этом загружаемая вещь не должна заботиться о том, откуда она берется, потому что вносить изменения проще. Все дело в ремонтопригодности.

Одним из примеров может быть загрузка шрифта из третьего источника (скажем, изображение спрайта символа). Для этого вам нужно изменить ваш загрузчик (чтобы вызвать третий метод, если первые два не пройдены) и сам класс Font для реализации этого третьего вызова. В идеале вы должны просто создать другую фабрику (SpriteFontFactory или что-то еще), реализовать тот же метод loadFont (...) и вставить его в список фабрик, которые можно использовать для загрузки шрифта.

Эд Джеймс
источник
1
Ах, я вижу: если я добавлю еще один способ загрузки шрифта, мне нужно будет добавить еще одну функцию получения для менеджера и еще одну функцию загрузки для ресурса. Действительно, это один недостаток. Однако, в зависимости от того, каким может быть этот новый источник, вам, вероятно, придется обрабатывать данные по-разному (TTF - это одно, а спрайты шрифтов - другое), поэтому вы не можете предсказать, насколько гибким будет определенный дизайн. Хотя я понимаю вашу точку зрения.
Пол
Да, как я уже сказал, мои навыки C ++ довольно ржавые, поэтому я изо всех сил пытался придумать жизнеспособную демонстрацию проблемы, я согласен с гибкостью. Это действительно зависит от того, что вы собираетесь делать с вашим кодом, как я уже сказал, я думаю, что ваш оригинальный код является вполне разумным решением проблемы.
Эд Джеймс
Отличный вопрос и отличный ответ, и самое лучшее, что несколько разработчиков могут извлечь из него уроки. Вот почему я люблю тусоваться здесь :). Да, и так, мой комментарий не является полностью избыточным, SRP может быть немного хитрым, потому что вы должны спросить себя «что, если», что может показаться противоречащим: «преждевременная оптимизация - корень всего зла» или « ЯГНИ 'Философия. Там никогда не черно-белый ответ!
Мартейн Вербург
0

Что меня беспокоит в вашем классе, так это то, что у вас есть loadFromMemoryи loadFromFileметоды. В идеале у вас должен быть только loadFromMemoryметод; шрифт не должен заботиться о том, как данные в памяти оказались. Другое дело, что вы должны использовать конструктор / деструктор вместо load и freeметодов. Таким образом, loadFromMemoryстало бы Font(const void *buf, int len)и free()стал бы ~Font().

zvrba
источник
Функции загрузки доступны из двух конструкторов, а в деструкторе вызывается free - я просто не показал этого здесь. Я считаю удобным иметь возможность загружать шрифт непосредственно из файла, вместо того, чтобы сначала открывать файл, записывать данные в буфер, а затем передавать его в Font. Иногда мне также нужно загрузить из буфера, поэтому у меня есть оба метода.
Пол