C ++ пример кодирования ужасов или блестящая идея? - PullRequest
12 голосов
/ 21 октября 2008

У предыдущего работодателя мы писали двоичные сообщения, которые должны были передаваться «по проводам» на другие компьютеры. Каждое сообщение имеет стандартный заголовок что-то вроде:

class Header
{
    int type;
    int payloadLength;
};

Все данные были смежными (заголовок, за которым сразу следуют данные). Мы хотели получить полезную нагрузку, учитывая, что у нас есть указатель на заголовок. Традиционно вы можете сказать что-то вроде:

char* Header::GetPayload()
{
    return ((char*) &payloadLength) + sizeof(payloadLength);
}

или даже:

char* Header::GetPayload()
{
    return ((char*) this) + sizeof(Header);
}

Это казалось многословным, поэтому я придумал:

char* Header::GetPayload()
{
    return (char*) &this[1];
}

Сначала это кажется довольно тревожным, возможно, слишком странным для использования - но очень компактным. Было много споров о том, было ли это гениальным или мерзостью.

Так что же это - преступление против кодирования или хорошее решение? У вас когда-нибудь был подобный компромисс?

-Обновление:

Мы попробовали массив нулевого размера, но в то время компиляторы давали предупреждения. В конце концов мы перешли к унаследованной технике: сообщение происходит от заголовка. На практике это прекрасно работает, но в принципе вы говорите сообщение IsA Header - что кажется немного неловким.

Ответы [ 17 ]

42 голосов
/ 21 октября 2008

Я бы пошел за преступление против кодирования.

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

Также обратите внимание, что метод НИЖЕ гарантированно работает. Sizeof () объекта включает отступы для выравнивания слов, так что если заголовок был:

class Header
{
    int type;
    int payloadLength;
    char  status;
};

Оба описанных вами метода будут иметь полезную нагрузку, начиная с заголовка + 12, тогда как, скорее всего, он действительно начинается с заголовка + 9.

14 голосов
/ 22 октября 2008

Вы зависите от компилятора для разметки ваших классов определенным образом. Я бы определил сообщение как структуру (со мной определяя макет) и имел бы класс, который инкапсулирует сообщение и предоставляет интерфейс для него. Чистый код = хороший код. «Милый» код = плохой (трудно поддерживаемый) код.

struct Header
{
    int type;
    int payloadlength;
}
struct MessageBuffer
{
   struct Header header;
   char[MAXSIZE] payload;
}

class Message
{
  private:
   MessageBuffer m;

  public:
   Message( MessageBuffer buf ) { m = buf; }

   struct Header GetHeader( )
   {
      return m.header;
   }

   char* GetPayLoad( )
   {
      return &m.payload;
   }
}

Прошло некоторое время с тех пор, как я написал C ++, поэтому прошу прощения за любые проблемы с синтаксисом. Просто пытаюсь донести общую идею.

14 голосов
/ 21 октября 2008

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

Но если вы собираетесь делать это таким образом, «this + 1» так же хорош, как и любой другой.

Обоснование: '& this [1]' - это фрагмент кода общего назначения, который не требует от вас копаться в определениях классов для полного понимания, и не требует исправления, когда кто-то меняет имя или содержимое класс.

Кстати, первый пример - настоящее преступление против человечества. Добавьте члена в конец класса, и он потерпит неудачу. Переместите участников по классу, и он потерпит неудачу. Если компилятор дополнит класс, он потерпит неудачу.

Кроме того, если вы собираетесь предполагать, что компоновка классов / структур компилятора соответствует компоновке вашего пакета, то вы должны понимать, как работает рассматриваемый компилятор. Например. на MSVC вы, вероятно, захотите узнать о #pragma pack.

PS: Немного страшно, сколько людей считают «это + 1» или «и это [1]» трудным для чтения или понимания.

13 голосов
/ 22 октября 2008

Это обычная проблема, но на самом деле вы хотите вот что.

class Header
{
    int type;
    int payloadLength;
    char payload[0];

};

char* Header::GetPayload()
{
    return payload;
}
5 голосов
/ 21 октября 2008

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

5 голосов
/ 21 октября 2008

Я думаю, что это некорректно с самого начала, если заголовок должен «возвращать» данные, которые не включены в него.

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

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

4 голосов
/ 22 октября 2008

Рассматривали ли вы трюк с "пустым членом массива"? Я помню, что видел его часто и даже использовал его один или два раза, но я не могу найти действительно хороших ссылок (кроме, может быть, того, на который ссылаются ниже).

Хитрость в том, чтобы объявить вашу структуру как

struct bla {
    int i;
    int j;
    char data[0];
}

Затем член 'data' просто указывает на то, что находится за заголовками. Я не уверен, насколько это портативно; Я видел это с '1' в качестве размера массива.

(использование URL-адреса, указанного ниже, в качестве ссылки, с использованием синтаксиса [1], похоже, не работает, поскольку он слишком длинный. Вот ссылка:)

http://developer.apple.com/documentation/DeveloperTools/gcc-4.0.1/gcc/Zero-Length.html

3 голосов
/ 21 октября 2008

Если это работает - последовательно - тогда это элегантное решение.

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

Я мог видеть, как это разваливается, когда объекты Header / Payload передаются «по проводам», потому что используемый вами механизм потоковой передачи, вероятно, не будет заботиться о выравнивании объектов на какой-либо конкретной границе. Таким образом, полезная нагрузка может следовать непосредственно за заголовком без заполнения, чтобы привести его к определенному выравниванию.

Чтобы придумать фразу, элегантный - это как элегантный. Так что это элегантно, если вы осторожны в том, как транслировать его.

2 голосов
/ 22 октября 2008

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

Является ли заголовок хранителем его полезной нагрузки?

Это фундаментальная проблема здесь - и заголовок, и полезная нагрузка должны управляться другим объектом, который содержит все сообщение, и это подходящее место для запроса полезной нагрузки. И это можно было бы сделать без арифметики указателя или индексации.

Учитывая это, я бы предпочел второе решение, так как оно более понятно, что происходит.

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

Если вы действительно хотите быть милым, вы можете обобщить.

template<typename T. typename RetType>
RetType JustPast(const T* pHeader)
{
   return reinterpret_cast<RetType>(pHeader + sizeof(T));
}
1 голос
/ 21 октября 2008

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

char* Header::GetPayload()
{
    return ((char*) this) + sizeof(*this);
}
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...