В последние месяцы я просил людей здесь, на 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...
}
Нижняя граница...
(Хотелось бы поставить здесь тролля, но я не могу вспомнить ни одного.: \)
Ну, у вас есть, я изложил свои аргументы как можно лучше. Пожалуйста, опубликуйте любые контраргументы, которые у вас есть, а также любые преимущества, которые, по вашему мнению, имеют предлагаемый дизайн по сравнению с моим собственным. По сути, попробуйте показать мне, что я не прав. :)
Ответы:
На мой взгляд, в этом коде нет ничего плохого, он делает то, что вам нужно, разумным и достаточно простым в обслуживании способом.
Однако проблема, с которой вы сталкиваетесь с этим кодом, состоит в том, что если вы хотите, чтобы он делал что-то еще, вам придется все это изменить .
Суть SRP заключается в том, что если у вас есть один компонент «CompA», который выполняет алгоритм A (), и вам нужно изменить алгоритм A (), вам также не нужно менять «CompB».
Мои навыки C ++ слишком проработаны, чтобы предложить достойный сценарий, в котором вам нужно будет изменить свое решение по управлению шрифтами, но я обычно делаю идею скользящего слоя кэширования. В идеале вы не хотите, чтобы загружающая вещь знала о том, откуда она берется, и при этом загружаемая вещь не должна заботиться о том, откуда она берется, потому что вносить изменения проще. Все дело в ремонтопригодности.
Одним из примеров может быть загрузка шрифта из третьего источника (скажем, изображение спрайта символа). Для этого вам нужно изменить ваш загрузчик (чтобы вызвать третий метод, если первые два не пройдены) и сам класс Font для реализации этого третьего вызова. В идеале вы должны просто создать другую фабрику (SpriteFontFactory или что-то еще), реализовать тот же метод loadFont (...) и вставить его в список фабрик, которые можно использовать для загрузки шрифта.
источник
Что меня беспокоит в вашем классе, так это то, что у вас есть
loadFromMemory
иloadFromFile
методы. В идеале у вас должен быть толькоloadFromMemory
метод; шрифт не должен заботиться о том, как данные в памяти оказались. Другое дело, что вы должны использовать конструктор / деструктор вместо load иfree
методов. Таким образом,loadFromMemory
стало быFont(const void *buf, int len)
иfree()
стал бы~Font()
.источник