Блог

Главная - Вопросы по программированию - Я сделал сомнительную вещь

Я сделал сомнительную вещь

#c #incomplete-type

#c #неполный тип

Вопрос:

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

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

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

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

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

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

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

Сомнительная вещь.Итак, GR имеет доступ к Font::data_ хотя friendship , но откуда он точно знает, что такое 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 Fontamp; fnt /* ... */)
{
    struct FontDataSurogate
        : public sf::Font {
    };

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

    // ...
}

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

Комментарии:

1. Я не понимаю «отдельный заголовок считался слишком большим количеством накладных расходов и имел мало практической пользы». Каковы накладные расходы? Я бы сказал, что это имеет большое практическое преимущество, поскольку вам не нужно объявлять структуру дважды и надеяться на правильный результат.

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

3. Я поддерживаю комментарий Джеймса. Самой сомнительной вещью, которую я увидел во всем посте, был аргумент против наличия .h файла, который определяет структуру данных, разделяемую между различными частями кода.

4. Вплоть до «сомнительной вещи» я обнаружил, что с облегчением киваю, соглашаясь со всеми вашими доводами. Это приятно видеть! В любом случае, я не совсем понимаю причину существования суррогата (обратите внимание: два r), но определение Font::FontData мне кажется хорошим.

5. О, теперь я понял. Я думаю, вам следовало написать (sf::Fontamp;)(*suro) вместо (sf::Font)(*suro) ; в противном случае, я согласен с другими: просто используйте заголовок. 🙂 Это будет достаточно инкапсулировано.

Ответ №1:

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

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

 struct FontDataSurogate
    : public sf::Font {
};

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

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

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

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

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

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

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

«Небольшое практическое преимущество» — printText () пришлось бы изменять каждый раз, когда изменяется FontData, независимо от того, определено ли это в отдельном заголовке или нет.

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

Ответ №2:

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

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

Ответ №3:

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

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

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

 class Font
{
  public:
    void do_something_with_fontdata(GraphicsRendereramp; gr);

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

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

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

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

Комментарии:

1. Инвертирование логики кажется мне немного обратным, кроме того, должен ли шрифт немного знать о реализации GR, чтобы это сработало? Думаю, в итоге у меня просто будет отдельный заголовок … 🙂

2. @Paul, да, это немного назад, но иногда это полезно. Да, шрифт должен был бы знать о деталях GR (возможно, вам нужно было бы создать сочетание методов, чтобы они могли общаться друг с другом). Если это звучит как слишком много работы, то, вероятно, так оно и есть. Не зная деталей вашего кода, я не могу сказать. В принципе, иногда это полезная техника, поэтому я хотел указать на нее. Отдельный файл заголовка, вероятно, лучший способ в данном случае, но вы искали другой способ.

Ответ №4:

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

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

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

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

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

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

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