How to define a well made message class in C++

Viewed 3509

At the moment I am working on a message class in C++ for data communication, e.g. over a serial port. For this question let's say I have two different messages (I do have more), e.g. a gyroscope- and a acceleration message both with calibration data and sensor values. Since I am an engineer who learned C++ for himself I had a look at my favorite C++ book and figured out that it might be useful to use a factory pattern for the message.

So a simplified version of my header file looks like this:

#ifndef MESSAGE_H
#define MESSAGE_H
#include <cstddef>

class message
{
public:

/**
 * \brief ~message:
 *        Virtual destructor of the class message.
 */
virtual ~message();

/**
 * \brief getMessage:
 *        Creates a message (with new).
 *
 * \param[in] size  Size of the dataArray.
 * \param[in] data  Bytearray of the message.
 *
 * \return If messagetype in the Array data eqal to 0 => new GyroMessage.
 *         If messagetype in the Array data eqal to 1 => new AccelMessage.
 *         Else => new NotValidMessage.
 *
 */
static message* getMessage(size_t size, char* data);

protected:

/**
 * \brief message:
 *        Default konstructor of the class message.
 *
 * \param[in] size  Size of the dataArray.
 * \param[in] data  Bytearray of the message.
 *
 */
message(size_t size, char* data);

/// Size of the dataArray.
int size;
/// Bytearray of the message.
char* dataArray;
/// Type of message.
char messagetype;
};

#endif // MESSAGE_H

The clases GyroMessage, AccelMessage and NotValidMessage are child classes of message. There are two things I do not like about this pattern:

First: If I want to add a new message, it is now enough to add a new class which is inherited form message, you have to add another if statement is in the static function getMessage.

Second: If I want to use the data e.g. from the GyroMessage, I have to reinterpret_cast the message.

Is there a better pattern that I could use for this purpose?

2 Answers

I'm not sure my ideas are good but it costs nothing sharing.

I've done something similar few days ago for Arduino, and for my case I made this choices:

  • for me the data payload is a pod (plain old data) struct where the compiler is informed to pack it. To achieve so I used a define that handles the compiler directive to pack it (for g++ is __attribute__((packed)))
  • the class in my case handles almost everything, thus I written it with a template. The template is for the payload struct
  • for sending the message in byte array I used a templated union inside the class. You have to check the byte ordering of the sender and receiver if you use this method. Something like that:

    template <class P>
      union Packet {
      P data;
      unsigned char buff[sizeof(P)];
    }
    

    My toolchain supports template so I used them. But you can use more template to insert more packages inside the same union. You need only to remember to have a common field in all the struct that identifies the kind of package (see below).

  • my class does not handles reading and writing, but instead requires the user of the class to provide 2 callbacks for reading and writing that accepts as arguments the buffer itself, the dimension of the buffer and a void pointer for user data. I think this is a good way to make the class agnostic with respect to the transmission channel.

The example (I'm simplifying a lot):

template <class A, class B,  std::size_t N>
union Packet {
  A a;
  B b;
  unsigned char buffer[N];
};

#define PAYLOAD(X, Y) struct X \
  Y __attribute__((packed));   \
  typedef struct X X;

template <class A, class B, std::size_t N>
class Message {
   union Packet<A, B, N> packet;

   // [...]
}

// [...]

// Time to declare you messages
PAYLOAD(GyroMessage, { char type; float x; float y; })
PAYLOAD(AccelMessage, { char type; float x; float y; float z; })
// GyroMessage will always have type = 0x01 and
// AccelMessage will always have type = 0x02 for example

// you know that sizeof(AccelMessage) > sizeof(GyroMessage)
// there is for sure a way to automatize this thing at 
// compilation time through macros. 
// Time to declare the class
Message<GyroMessage, AccelMessage, sizeof(AccelMessage)> message;

To retrieve the correct value you can still (through the union) access the value that you want, without adding new particular functions. But at the moment I don't have a very nice way to access them programmatically.

Those are only some hints, I really don't know what you are doing or which are your priority. So take them with caution.

A factory function is an excellent solution when you want to work generically, with knowledge of which type to create either inferred or known only at the point of creation.

If you know what type you want to create, and you are planning to use the created class with a pointer to the specific, derived type (using its specific interface) then you do not need a factory function. Create what you need and use it! In that case, you need a base class only for common functionality and/or for passing to specific functions which work on all derived types generically.

Alternatively, if you do want to work generically, then you can use a factory function and not cast the result. Work with a pointer to the base class. Have a common interface and handle the differences internally. If you can not do this, then your scenario is not a candidate for this approach.

Related