Я сделал сомнительную вещь - PullRequest
5 голосов
/ 09 апреля 2011

Являются ли (казалось бы) тенистые вещи приемлемыми по практическим соображениям?

Сначала немного информации о моем коде. Я пишу графический модуль моей 2D-игры. Мой модуль содержит более двух классов, но здесь я упомяну только два: Шрифт и GraphicsRenderer .

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

class Font
{
  private:
    struct FontData;
    boost::shared_ptr<FontData> data_;
};

GraphicsRenderer - это устройство (читай: singleton), которое инициализирует и финализирует стороннюю графическую библиотеку, а также используется для визуализации графических объектов (таких как как Шрифты , Изображения и т. д.). Причина, по которой он является единичным, заключается в том, что, как я уже сказал, класс автоматически инициализирует стороннюю библиотеку; это происходит при создании одноэлементного объекта и выходит из библиотеки при его уничтожении.

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

Примечание: до сих пор я делал две вещи, которые некоторые могут считать неясными (синглтон и друг), но я не хочу об этом спрашивать. Тем не менее, если вы считаете, что мое обоснование превращения GR в синглтон и друга Font неверно, пожалуйста, критикуйте меня и, возможно, предложите лучшие решения.

Тенистая вещь. Итак, GR имеет доступ к Font :: data _ хотя и дружба, но как он точно знает, что такое FontData есть (так как он не определен в заголовке, это неполный тип)? Я просто покажу код и комментарий с обоснованием ...

// =============================================================================
//   graphics/font.cpp
// -----------------------------------------------------------------------------

struct Font::FontData
    : public sf::Font
{
    // Just a synonym of sf::Font
};

// A redefinition of FontData exists in GraphicsRenderer::printText(),
// which will have to be modified as well if this definition is modified.
// (The redefinition is called FontDataSurogate.)
// Why not have FontData defined only once in a separate header:
// If the definition of FontData changes, most likely printText() text will
// have to be altered also regardless. Considering that and also that FontData
// has (and should have) a very simple definition, a separate header was
// considered too much of an overhead and of little practical advantage.


// =============================================================================
//   graphics/graphics_renderer.cpp
// -----------------------------------------------------------------------------

void GraphicsRenderer::printText(const Font& fnt /* ... */)
{
    struct FontDataSurogate
        : public sf::Font {
    };

    FontDataSurogate* suro = (FontDataSurogate*)fnt.data_.get();
    sf::Font& font = (sf::Font)(*suro);

    // ...
}

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

Ответы [ 4 ]

3 голосов
/ 09 апреля 2011

В общем, если что-то выглядит отрывочно, я обнаружил, что часто стоит несколько раз вернуться назад и попытаться выяснить, почему это необходимо.В большинстве случаев всплывает какое-то исправление (может быть, не такое «приятное», но не полагающееся на какие-то хитрости).

Итак, первая проблема, которую я вижу в вашем примере, - это немного кода:

struct FontDataSurogate
    : public sf::Font {
};

встречается дважды в разных файлах (ни один из них не является заголовком).Это может вернуться и будет беспокоить, когда вы в будущем поменяете одно, а не другое, и очень вероятно, что убедиться в том, что оба они идентичны, будет боль.

Чтобы решить эту проблему, я бы предложил поместить определение вFontDataSurogate и соответствующий включает (независимо от того, какую библиотеку / заголовок определяет sf::Font) в отдельный заголовок.Из двух файлов, которые должны использовать FontDataSurogate, включите этот заголовок определения (не из каких-либо других файлов кода или заголовков, а только эти два).

Если у вас есть заголовок объявления основного класса для вашей библиотеки, поместитетам есть прямое объявление для класса и использование указателей в ваших объектах и ​​параметрах (обычные указатели или общие указатели).

Затем можно использовать friend или добавить метод get для извлечения данных, но, перемещаяопределение класса для его собственного заголовка, вы создали единственную копию этого кода и имеете один объект / файл, который взаимодействует с другой библиотекой.

Редактировать: Вы прокомментировалиПока я писал этот вопрос, я добавлю ответ на ваш комментарий.

«Слишком много накладных расходов» - больше документов, еще одна вещь, которую нужно включить, сложность кода возрастаети т. д.

Не так.У вас будет одна копия кода, по сравнению с двумя, которые должны остаться идентичными сейчас.Код существует в любом случае, поэтому он должен быть задокументирован, но ваша сложность и, в частности, обслуживание упрощены.Вы получаете два #include заявления, но так ли это дорого?

"Маленькое практическое преимущество" - printText () придется изменять каждый раз, когда FontData изменяется независимо от того, изменен он или нетон определен в отдельном заголовке или нет.

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

2 голосов
/ 09 апреля 2011

friend в порядке и приветствуется. См. C ++ FAQ Обоснование Lite для получения дополнительной информации: Друзья нарушают инкапсуляцию?

Эта строка действительно ужасна, так как вызывает неопределенное поведение : FontDataSurogate* suro = (FontDataSurogate*)fnt.data_.get();

0 голосов
/ 09 апреля 2011

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

Вы указываете три причины, по которым вы не хотите добавлять файл:

  1. Дополнительно включают
  2. Дополнительная документация
  3. дополнительная сложность

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

Единственное, что вы сохраняете, это включаемый файл. Но файлы дешевые. Вы не должны бояться создавать их. Стоимость добавления нового файла заголовка практически равна нулю (или, по крайней мере, должна быть).

Преимущества правильного выполнения:

  1. Компилятор не должен делать совместимое определение, которое вы даете
  2. Когда-нибудь кто-то собирается изменить класс FontData без изменения PrintText (), возможно, им следует изменить PrintText (), но они либо еще не сделали этого, либо не знают, что им нужно. Или, может быть, таким способом, который просто не обнаружил дополнительных данных в FontData, имеет смысл. Независимо от того, разные куски кода будут работать с различными предположениями и будут очень трудно отследить ошибку.
0 голосов
/ 09 апреля 2011

Вы объявляете о существовании структуры FontData, а затем полностью объявляете ее в двух местах: Font и GraphicsRenderer. Еа. Теперь вы должны вручную сохранить эти данные в двоичном формате.

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

Один из методов - инвертировать вашу обработку. Вместо того, чтобы помещать всю логику в GraphicsRenderer, поместите некоторую ее часть в Font. Вот так:

class Font
{
  public:
    void do_something_with_fontdata(GraphicsRenderer& gr);

  private:
    struct FontData;
    boost::shared_ptr<FontData> data_;
};

void GraphicsRenderer::printText(const Font& fnt /* ... */)
{
   fnt.do_something_with_fontdata(*this);
}

Таким образом, детали Font хранятся в классе Font, и даже GraphicsRenderer не должен знать специфику реализации. Это также решает проблему friend (хотя я не думаю, что другом все-таки плохо пользоваться).

В зависимости от того, как устроен ваш код и что он делает, попытка инвертировать его таким образом может быть довольно сложной. Если это так, просто переместите реальное объявление FontData в его собственный заголовочный файл и используйте его в Font и GraphicsRenderer.

...