Search code examples
c++architectureversiondriver

Code structure for different versions of devices


I am writing a "device driver" (C++14) which can handle multiple versions of protocols meant for different versions of devices. This device driver is running on an external PC which communicates with the device over Ethernet with a HTTP based protocol. There are common functionalities for all versions, but some functions maybe additional in certain versions of the protocol.

Below is an example:

class ProtocolBase {
public:
    virtual void reset_parameters() {
        std::cout << "reset parameters" << std::endl;
    }

    virtual void set_parameters() {
        std::cout << "set parameters" << std::endl;
    }
};

class ProtocolV1 : public ProtocolBase
{
public:
    void set_parameters() override {
        std::cout << "set parameters for V1" << std::endl;
    }
};

class ProtocolV2 : public ProtocolBase 
{
public:
    void set_parameters() override {
        std::cout << "set parameters for V2" << std::endl;
    }

    void reset_parameters() {
        std::cout << "reset parameters for V2" << std::endl;
    }

    void do_V2() {
        std::cout << "doing V2" << std::endl;
    }
};

Below is the main:

int main(int argc, char const *argv[])
{
    int version = std::atoi(argv[1]);

    std::unique_ptr<ProtocolBase> protocol = std::make_unique<ProtocolV1>();
    switch (version)
    {
    case 1:
        /* do nothing at the moment */
        break;
    case 2:
        protocol.reset(new ProtocolV2);
        break;
    default:
        break;
    }

    protocol->reset_parameters();

    if(ProtocolV2* p = dynamic_cast<ProtocolV2*>(protocol.get())) { //not sure about this
        p->do_V2();
    }else {
        std::cout << "This functionality is unavailable for this device" << std::endl;
    }
    protocol->set_parameters();
    return 0;
}

I have a feeling using dynamic_cast is not the best way to go here. Looking forward to some feedback.

Edit: As per @Ptaq666's answer, I modified ProtocolBase and ProtocolV2 as:

class ProtocolBase {
public:
    virtual void do_V(){
        std::cerr << "This functionality is unavailable for this device" << std::endl;
    }
};
class ProtocolV2 : public ProtocolBase 
{
public:
    void do_V() override {
        std::cout << "doing V2" << std::endl;
    }
};

With this, there's no need for dynamic_cast anymore, though base class will have to know all the functionalities. This seems to be the best solution for now.


Solution

  • Like in most cases when it comes to chose the appropriate system architecture the answer is "it depends" :). The most comfortable solution would be to introduce protocol-specific behavior of the ProtocolBase subclasses in their constructors

    class ProtocolV2 : public ProtocolBase 
    {
    public:
        ProtocolV2::ProtocolV2(args) {
            // set some params that will determine when do_V2() is called
            // it can be some numeric setting, a callback, or similar
        }
        void set_parameters() override {
            // you can use V2 behavior here maybe?
            std::cout << "set parameters for V2" << std::endl;
        }
    
        void reset_parameters() override {
            // or here maybe?
            std::cout << "reset parameters for V2" << std::endl;
        }
    
    private:
        void do_V2() {
            std::cout << "doing V2" << std::endl;
        }
    };
    

    If for some reason you cannot do this, there is a possibility to keep do_V2() as public non-virtual method, but to call it before passing ProtocolV2 as a pointer to ProtocolBase to the sysytem that will use it. Of course the limitation is that do_V2 can only be called outside your system scope, which might not really solve the problem.

    Another option is to actually move do_V2() to the interface:

    class ProtocolBase {
    public:
        virtual void reset_parameters() {
            std::cout << "reset parameters" << std::endl;
        }
        virtual void set_parameters() {
            std::cout << "set parameters" << std::endl;
        }
        virtual void do_V2() {
            std::cout << "not supported" << std::endl;
        }
    };
    

    and implement it as "not supported" behavior by default. Only ProtocolV2 will implement this behavior as a valid part of the protocol.

    In the end, if none of the above is OK, you can of course use the dynamic_cast as you proposed. Personally I try to avoid dynamic_cast because my office mates will start to abuse it for sure, but in some cases it is a correct solution.

    Also if you decide to cast the pointer, use std::shared_ptr with dynamic_pointer_cast instead of accessing a raw pointer from unique_ptr.