Best practices to implement a Payload-containing class in C++?

Question

I have a question about hierarchy, references and pointers... The question comes to my mind when I had tried to do the following stuff:

class packet {
public:
    int address;
    int command; /**< Command select the type of Payload that I must decode */
    Payload p; /**< Generic payload, first question:
                    Payload p or Payload * p or Payload &p ?
                    I have a background in C, for this reason I prefer
                    Payload p but I know that this is not recommended for C++ */

private:
    /** All getter and setter for attributes */
    /** Second question: What is the best way to implement a getter
        and setter for Payload?... I prefer something
        similar to Java if this is possible */    
}

Now imagine that I have a lot of types of Payload, all these payloads are children of the super class (generic) Payload.

I want to read the header and switch o the command. For example, if command is 1 I create a PayloadReset : Payload and fill in all of its attributes, then I want to set on my packet this payload (up-casting). In other part of the program I want to read my current packet and then read the command field and down-cast to the appropriate type depending on the command field.

When I tried to do this, I could do the up-casting without problems but the problem comes when I tried to do the downcasting to the specific Payload, in our example PayloadReset.

Answer 1

To answer the first question (which was buried inside the comments in your first code example:

Payload *p;

The first thing you need to learn as part of your transition from Java to C++ is what pointers are and how they work. What will be confusing to you, for some time, is the fact that all objects in Java are really pointers. You never needed to know that, when working with Java. But you must know that now, in order to understand C++. So, declaring a C++ class as

Payload p;

Is not the same thing as making a similar declaration in Java. There is no equivalent to this declaration in Java. In Java you really have a pointer here, and you have to instantiate it using the new keyword. That part Java originally aped from C++. This is the same process as C++, except that you have to explicitly declare it as a pointer.

Payload *p;

Then, somewhere else, using your example of a PayloadReset subclass:

class PayloadReset : public Payload { /* Class declaration */ };

PayloadReset *r = new PayloadReset( /* Constructor argument */ };

p=r;

And the second thing you need to learn as part of your transaction from Java to C++ is when, and how, to delete all instantiated objects. You don't have Java's garbage collector here. This becomes your job, now.

Answer 2

Your question is somewhat related to Java syntax, but mostly about Object Oriented Programming.

First of all, you should take a moment to get familiar with Java naming conventions. There are commonly used recommendations that you can find all over the web. Here is one example of Java Naming Conventions. I brought this up because single variable names is generally not a good idea and having descriptive variables names pays dividends as the program grows in size and especially if there are more than one person on a team. So, instead of Payload p use Payload payload.

Secondly, in OO (Object Oriented), it is best to always keep your Class instance variables private, not public. Give access to these variables only if necessary and shield access to them by providing public methods. So, in your example of class Packet, your public/private is backwards. Your class should look more like:

public class Packet{
    //private fields
    private int address;
    private int command;
    private Payload payload;

    //Maybe provide a nice constructor to take in expected
    //properties on instantiation
    public Packet(Payload pay){

    }

    //public methods - as needed
    public void getPayload(){
       return this.payload;
    }

    public void setAddress(int addy){
        this.address = addy;
    }

    public int getCommand(){
       return this.command;
    }
}

Also, to answer more of your question about the naming of Payload. Like i said earlier..use descriptive names. Java does not have pointer references like C and generally handles memory management for you, so the & is not required or supported.

Your last question/topic is really again about OO and Class heirarchy. It seems that Payload would be a generic base class and you may have multiple, specific 'Payload types', like ResetPayload. If that is the case, you would then define Payload and create the ResetPayload class that extends Payload. I'm not sure exactly what you are trying to do, but think of Classes/objects ad nouns and methods as verbs. Also think about the 'is-a' and 'has-a' concept. From what I see, maybe all Payloads 'has-acommand and an address. Also, maybe eachPayloadalso has multiplePackets, whatever. Just as an example, you would then define yourPayload` class like this:

public class Payload{
    private int address;
    private int command;
    private List<Packet> packets = new ArrayList<>();

    public Payload(int addy, int comm){
        this.address = addy;
        this.command = comm;
    }

    public void addPacket(Packet p){
        packets.add(p);
    }

    public List<Packet> getPackets(){
        return this.packets;
    }

    public int getCommand(){
        return this.command;
    }

    public int getAddress(){
        return this.address;
    }
}

Then if you had a type of Payload that is more specific, like Reset, you would create the class, extends Payload and provide the additional properties/operations specific to this type, something this like:

public class ResetPayload extends Payload{

     public ResetPayload(int addy, int comm){
         super(addy, comm);
     }

     public void reset(){
         //Do stuff here to reset the payload
     }
}

Hopefully, that answers your questions and moves you along further. Good luck.

Answer 3

Tagging onto Sam's answer.

1. Before you go any further, learn the difference between stack and heap allocation. In the example you posted, you're allocating your Payload p; object on the stack - implying that the size of the object is known at this point and said size will be allocated on the stack. If you wanted to assign an derived object to p, it wouldn't work, because said object will likely be of different size. This is why you instead declare a pointer to the object (8 bytes on 64-bit architecture, 4 bytes on 32 bit), and then when you know which type of derived object you want to allocate, you do it using the new operator, as such:

   Payload *p;
   p = new PayloadReset(...);
   

2. The above method would require manually managing memory, i.e. calling delete on the new allocated pointer. As of C++11, the recommendation is to use smart pointers from the <memory> header. These are essentially reference counted pointers that automatically call delete for you.

   std::shared_ptr<Payload> p;
   p = std::make_shared<PayloadReset>(...);
   

Answer 4

Here is my take on the general problem, it extends the tagged union idea. Advantages are 1.) no inheritance/dynamic_cast 2.) no shared ptr 3.) POD 4.) rtti is used to generate unique tags:

using cleanup_fun_t = void(*)(msg*);

class msg
{
public:
    template<typename T, typename... Args>
    static msg make(Args&&... args);
private:
    std::type_index tag_;
    mutable std::atomic<cleanup_fun_t> del_fn_; // hell is waiting for me, 
    uint64_t meta_;
    uint64_t data_;
};

Please fill in all the nice member functions. This class is move only. You are creating messages with payload by the static member function make:

template<typename T, typename... Args>
msg msg::make(Args&&... args)
{
     msg m;
     m.tag_ = typeid(T);
     m.del_fn_ = nullptr;
     if (!(std::is_empty<T>::value))
     {
         auto ptr = std::make_unique<T>(std::forward<Args>(args)...);
         m.data_ = (uint64_t)ptr.release();
         m.del_fn_ = &details::cleanup_t<T>::fun; // deleter template not shown
     }
     return m;
 }

 // creation:
 msg m = msg::make<Payload>(params passed to payload constructor);
 // using
 if (m.tag() == typeid(Payload))
 {
     Payload* ptr = (Payload*)m.data;
     ptr-> ...
 }

Just check the tag if it contains your expected data (type) and cast the data to a pointer type.

Disclaimer: It is not the complete class. Some access member function are missing here.