jeudi 1 août 2019

C++: Base class calling own virtual function - an anti-pattern?

I see pattern like below commonly used. We have a base class which does most of the work, but calls one of its own virtual/pure-virtual function to do the part of the job which is different for each derived type. A contrived example:

struct PacketProcessor {
    virtual void parseEncap(pkt) = 0;
    void process(Pkt pkt)
    {
        parseEncap(pkt);  // Calls its own virtual function to parse the encap
        processFurther(pkt);
        ...
    }
};

We create derived classes which will override the virtual functions and provide functionality specific to derived classes.

struct EthernetProcessor : public PacketProcessor {
    void parseEncap(Pkt) override { // parse ethernet encap}
};

struct PPPProcessor : public PacketProcessor {
     void parseEncap(Pkt) override { //parse ppp encap }
};

But I feel with this sort of pattern, as time goes on, more and more functions in the base class gets virtualized or more virtual function added and called at random places to make room for different derived class behavior.

[In a real life code I have see an add() and add_extra() virtual functions :-) ]

And over time the code does not have a solid structure any more as each type is handled in totally different ways. Even though the common base class sort of gives a false notion a structure. But yes it still keeps code for different types segregated as opposed to if (type1)/else(type2).

Another way of achieving something similar is to abstract out the differences into a different class (hierarchy) and call the virtual functions in that. This is also very common For Eg:

struct Encap {
    virtual void parseEncap(Pkt) = 0;
};

struct EthernetEncap : public Encap {
    void parseEncap(Pkt pkt) {}
};

struct PPPEncap : public Encap {
    void parseEncap(Pkt pkt) {}
};

struct PacketProcessor {
    PacketProcessor(Encap *encap) : m_encap{encap} {}
    void process(Pkt pkt)
    {
       m_encap->parseEncap(pkt);
       processFurther(pkt);
       ...
    }
private:
    EncapPtr m_encap;
};

But in this case also there can be frivolous functions added the Encap class. Or there will too many component classes like the Encap providing different functionality.

But the good thing is that the PacketProcessor will follow a specific path for all types of Encaps. Also this approach is not as flexible as the former pattern because we need to put all 'Encap's into a very specific mold.

So my question is:

Is either one of these an anti-pattern and should avoided or drawbacks simply lack of discipline and nothing to do with the pattern followed.

Aucun commentaire:

Enregistrer un commentaire