Decoding and Encoding packet layers using decorator pattern I am trying to use decorator pattern to decode and encode packet layers in my application. I've supposed my packet has three layers: Header, Level2, Level3. So far I've come up with the following code. For the sake of simplicity, I've supposed that the data of each layer is the first 10 bytes of the input packet. I've also intentionally ignored memory leak problems.

#include &lt;iostream&gt;
#include "Decoder.h"

int main()
{
    Header* header = new Header("HeaderData");
    Level2* l2 = new Level2(header, "Level2Data");
    Level3* l3 = new Level3(l2, "Level3Data");

    std::cout &lt;&lt; header-&gt;Encode() &lt;&lt; std::endl;
    std::cout &lt;&lt; l2-&gt;Encode() &lt;&lt; std::endl;
    std::cout &lt;&lt; l3-&gt;Encode() &lt;&lt; std::endl;

    std::string packet = l3-&gt;Encode();

    header-&gt;Decode(packet);
    l2-&gt;Decode(header-&gt;mNextData);
    l3-&gt;Decode(l2-&gt;mNextData);

    std::cout &lt;&lt; ((Header*)header)-&gt;mData &lt;&lt; std::endl;
    std::cout &lt;&lt; ((Level2*)l2)-&gt;mData &lt;&lt; std::endl;
    std::cout &lt;&lt; ((Level3*)l3)-&gt;mData &lt;&lt; std::endl;
}


Decoder.h file:

#ifndef _DECODER_H_
#define _DECODER_H_

#include &lt;string&gt;

class DecoderBase
{
public:
    virtual void Decode(std::string &amp;packet) = 0;
    virtual std::string Encode() = 0;
protected:
    DecoderBase* mInnerLayer;
};

class Header : public DecoderBase
{
public:
    Header(const std::string &amp;data);
    virtual ~Header();
    virtual void Decode(std::string &amp;packet);
    virtual std::string Encode();
    std::string mData;
    std::string mNextData;
};

class Level2 : public DecoderBase
{
public:
    Level2(DecoderBase* innerLayer, const std::string &amp;data);
    ~Level2();
    virtual void Decode(std::string &amp;packet);
    virtual std::string Encode();
    std::string mData;
    std::string mNextData;
};

class Level3 : public DecoderBase
{
public:
    Level3(DecoderBase* innerLayer, const std::string &amp;data);
    ~Level3();
    virtual void Decode(std::string &amp;packet);
    virtual std::string Encode();
    std::string mData;
    std::string mNextData;
};

#endif


Decoder.cpp file:

#include "Decoder.h"

Header::Header(const std::string &amp;data)
{
    mData = data;
}

Header::~Header(){}

void Header::Decode(std::string &amp;packet)
{
    mData = packet.substr(0, 10);
    mNextData = packet.substr(10);
}

std::string Header::Encode()
{
    return mData;
}

Level2::Level2(DecoderBase *innerLayer, const std::string &amp;data)
{
    mInnerLayer = innerLayer;
    mData = data;
}

void Level2::Decode(std::string &amp;packet)
{
    mData = packet.substr(0, 10);
    mNextData = packet.substr(10);
}

std::string Level2::Encode()
{
    return mInnerLayer-&gt;Encode() + mData;
}

Level3::Level3(DecoderBase *innerLayer, const std::string &amp;data)
{
    mInnerLayer = innerLayer;
    mData = data;
}

void Level3::Decode(std::string &amp;packet)
{
    mData = packet.substr(0, 10);
    mNextData = packet.substr(10);
}

std::string Level3::Encode()
{
    return mInnerLayer-&gt;Encode() + mData;
}


Output:


  HeaderData
  HeaderDataLevel2Data
  HeaderDataLevel2DataLevel3Data
  HeaderData
  Level2Data
  Level3Data  



Am I using the decorator pattern correctly to decode and encode packets?
Who is responsible for calling the Decode method of the next layer? Am I calling the Decode method of each layer correctly? I thought maybe l2-&gt;Decode() should be called from within header-&gt;Decode().


Any advice on improving the above code would be appreciated!
I don't know about the decorator pattern, but there are things you can improve anyway:


Identifiers that begin with an underscore followed by a capital letter are reserved identifiers in C++. Therefore, _DECODER_H_ is not a valid identifier; that name is reserved to the implementation. You should change it to DECODER_H_.
You never modify the string that you pass to Decode. Therefore, you should pass it a const reference instead of a simple non-const one:

virtual void Decode(const std::string &amp;packet) = 0;

All of your class members are public. Therefore, you could simply change them to structs instead and remove all the public qualifications. The only difference between class and struct is that all is public by default (even inheritance) while all is private by default within a class.
You don't seem to need pointers in main, regular automatic variables would be enough. Actually, try to use pointers only when you have to in C++, only where it makes sense. Otherwise, try to always use value and/or reference semantics. That way, you may avoid many problems.
DecoderBase is probably meant to be a base class to be used for some polymorphic behaviour. Give it a virtual destructor. Otherwise, deleting a derived instance through a base pointer will be undefined behaviour.
Try to use constructor initialization lists whenever possible:

Level2::Level2(DecoderBase *innerLayer, const std::string &amp;data):
    mInnerLayer(InnerLayer),
    mData(data)
{}

There are several functions that could be const-qualified (try to do so whenever a function does not modify the members of the class). The Encode methods look like they should be const-qualified everywhere:

std::string Header::Encode() const
{
    return mData;
}