Code structure for different versions of devices

Question

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.

Answer 1

In general it depends on how the derived classes ProtocolV1 and ProtocolV2 are formed and what are the data members and weather if the respective member functions are going to use different data members or not!

The reason is simply since there is no dependency to member data, the member functions are only sensitive to the type of the objects that they have been called with, not their value/state!

It is like having a (non member) function overload like:

void function(ProtocolV1 *){
        std::cout << "set parameters for V1" << std::endl;
}
void function(ProtocolV2 *){
        std::cout << "set parameters for V2" << std::endl;
}

And then calling it once by a pointer of type ProtocolV1 * and once with null pointer of typeProtocolV2 *.

If you like alternatives for the usage you presented in the question you can even use C-style cast: The result was the SAME!

Finally if you are going to call the member function to then call another function from it which requires some data member/s ,which is/are different across the derived classes as its argument/s, then you can not use any cast unless you introduce some form of compensation to fill the data that is not presented in the casted type!

Good luck!

Answer 2

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.