Search code examples
c++polymorphismc++17dynamic-dispatch

Polymorphic member variable(s) - class design


Wondering whether anyone can help identify a more elegant design approach - or potentially identifying shortcomings of the following design.

Currently, I have an abstract Response class that derives from a serializable JSON Object.

//objects.h
struct Object
{
    [[nodiscard]] std::string serialize() const;

    virtual void deserialize(const Poco::JSON::Object::Ptr &payload) = 0;

    [[nodiscard]] virtual Poco::JSON::Object::Ptr to_json() const = 0;
};
// response.h
class Response : public Object
{
public:
    std::unique_ptr<Data> data;
    std::unique_ptr<Links> links;
};

Where both Data and Links member variables are abstract base classes - in which their respective set of subclasses contain various STL containers.

Now the problem I'm facing is one of class design - and how to avoid downcasting each member variable depending on the derived Response (and to identify a more clean hierarchy/design). For instance...

ResponseConcreteA response_a;
response_a.deserialize(object_a);
auto data_a = static_cast<DataConcreteA *>(response_a.data.get());

ResponseConcreteB response_b;
response_b.deserialize(object_b);
auto data_b = static_cast<DataConcreteB *>(response_b.data.get());

The seemingly obvious solution is to abandon polymorphic member variables and substitute them for the respective concrete types. However - my concern is that this is a deviation from the inherent relationship of a Response having Data & Links members which are each a particular polymorphic type.

One important thing to note is that the concrete types attributed to Data & Links are determined at compile time - there is no necessity for the derived classes to change at any point. There respective construction(s) is governed by the following preprocessed template:

#define DECLARE_RESPONSE_TYPE(type_name, data_name, links_name \
        struct type_name final : public Response \
        { \
            type_name() \
            { \
                data.reset(new data_name()); \
                links.reset(new links_name()); \
            } \
            ~type_name() = default; \
            void deserialize(const Poco::JSON::Object::Ptr &payload) override; \
            Poco::JSON::Object::Ptr to_json() const override; \
        };

Is there a more appropriate approach to avoid these polymorphic member variables in my design where constant downcasting is required (despite the fact that derived object pointed to is known at compile time). Thanks!


Solution

  • (I’m adapting one of my recent Reddit comments that answered basically the same question.)

    in general

    Don’t model serialization with inheritance! It’s a cross-cutting concern you want to attach to arbitrary types. Inheritance is the wrong tool for that. Some problems with the approach:

    • You force everything serializable to become a full-fledged polymorphic class with all the related overhead.
    • You need control over the type you want to serialize, which means you cannot use 3rd party types without wrapping them.
    • Because serialization is cross-cutting you’ll likely run into the inheritance diamond problem at some point.
    • Fundamental types cannot be serialized that way. You cannot make int derive from Serializable.

    Pattern matching is a more flexible approach. In a nutshell, you template your serialization framework and depend on certain functions being available for serializable types. Quick, dirty and naive example:

    struct Something {
        // ...
    };
    
    // If these two functions exist a type has serialization support
    void serialize(const Something&, SerializedDataStream&);
    Something deserialize(SerializedDataStream&);
    

    Now you can make anything serializable without touching the type at all. That’s vastly more flexible than inheritance, but probably makes the serialization framework somewhat trickier to implement. Additionally supporting (de)serialization member functions is a good idea for more complex types that need access to their private data to (de)serialize properly.

    Have a look at Boost Serialization or Cereal for real world examples of the pattern matching approach.

    in your particular situation

    To serialize larger nested structures you split up the serialization functionality. Each type has to know how to serialize itself, but that’s as far as it goes. Serializing a complex member is delegated to that member, because it too has to know how to serialize itself. That way you build the final JSON step by step.

    One important thing to note is that the concrete types attributed to Data & Links are determined at compile time

    The obvious solution is to turn Response into a template.

    template <typename Data, typename Links>
    class Response  // note: no more base class
    {
    public:
        Data data;
        Links links;
    };
    
    // externalized serialization functions
    void serialize(const Response&, JSONDataStream&);
    Response deserialize(JSONDataStream&);
    

    That way you have the correct types available to find the correct overload of the serialization functions for Data and Links, and delegation boils down to simply call them. Whether that approach is feasible depends on the larger context. Retrofitting a template into a project that relies on polymorphism can lead to ripple effects throughout the whole code base. In other words, it can be a really expensive change.

    The alternative is similar to what you’re already doing. Response itself still uses the pattern-matching approach to serialization. But you keep the polymorphism for Data and Links including the overriden virtual serialization functions. In each concrete derived type we’re back to the original idea of “each type knows how to serialize itself”. If the concrete Data and Links classes need to be serialized in other contexts (not as members of Response), too, implement the pattern-matching functions for them and call those from the overriden member functions. Otherwise serialization can happen directly in those member functions.

    class Data
    {
    public:
        virtual ~BaseData() = default;
        
        void deserialize(const Poco::JSON::Object::Ptr &payload) = 0;
        Poco::JSON::Object::Ptr to_json() const = 0;
        
        //...
    };
    
    class ConcreteData
    {
    public:
        ~BaseData() override = default;
        
        void deserialize(const Poco::JSON::Object::Ptr &payload)
        {
            // ...
        }
        
        Poco::JSON::Object::Ptr to_json() const
        {
            // ...
        }
    }
    
    // ------
    
    Poco::JSON::Object::Ptr Response::to_json() const
    {
        // ...
        
        auto serializedData = data->to_json();
        
        // ...
    }