Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C++ example of Coding Horror or Brilliant Idea?

At a previous employer, we were writing binary messages that had to go "over the wire" to other computers. Each message had a standard header something like:

class Header
{
    int type;
    int payloadLength;
};

All of the data was contiguous (header, immediately followed by data). We wanted to get to the payload given that we had a pointer to a header. Traditionally, you might say something like:

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

or even:

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

That seemed kind of verbose, so I came up with:

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

It seems rather disturbing at first, possibly too odd to use -- but very compact. There was a lot of debate on whether it was brilliant or an abomination.

So which is it - Crime against coding, or nice solution? Have you ever had a similar trade-off?

-Update:

We did try the zero sized array, but at the time, compilers gave warnings. We eventually went to the inhertited technique: Message derives from Header. It works great in practice, but in priciple you are saying a message IsA Header - which seems a little awkward.

like image 530
Dan Hewett Avatar asked Oct 21 '08 19:10

Dan Hewett


2 Answers

I'd go for crime against coding.

Both methods will generate the exact same object code. The first makes it's intention clear. The second is very confusing, with the only advantage that it saves a couple keystrokes. (Just learn to freakin' type).

Also, note that NEITHER method is guaranteed to work. The sizeof() an object includes padding for word alignment, so that if the header was:

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

Both methods you describe would have the payload starting at Header+12, when most likely it actually starts at Header+9.

like image 200
James Curran Avatar answered Oct 27 '22 02:10

James Curran


You're depending on the compiler to layout your classes in a particular way. I would have defined the message as a struct (with me defining layout) and had a class that encapsulates the message and provides the interface to it. Clear code = good code. "Cute" code = bad (hard to maintain) code.

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

It's been awhile since I've written any C++, so please excuse any issues with syntax. Just trying to convey the general idea.

like image 29
tvanfosson Avatar answered Oct 27 '22 04:10

tvanfosson