Return to Answer

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

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