CODEWITHSUNDEEP
c++oopdesign-patternspolymorphism
Vlad Lazar
Vlad Lazar

Reputation: 387

Is it ok to keep adding virtual methods to the base class(the interface) as needed?

Let us consider the following OOP structure. The base class(interface) is the so called 'Node' class. Two other classes inherit from 'Node', namely 'ElementNode', 'TextNode'. I need to use a polymorphic container(vector>). At some point I need to get some class member from an element extracted via vector, which is hidden by the upcasting, for example text from 'TextNode'. My question is if, design wise, it is fine to keep adding virtual methods to the base class 'Node' in order to gain access to the hidden members. Another option would be to make use of down casting. Is the design flawed and if it is what alternative would you suggest?

Example code in C++:

class Node {
public:
     Node() {};
     virtual ~Node() {};

     NodeTypes getNodeType() const { return nodeType; };
     virtual std::vector<std::shared_ptr<Node>> &getChildren() = 0;
     virtual void pushChild(std::shared_ptr<Node>) = 0;

private:
     NodeTypes nodeType;
     std::vector<std::shared_ptr<Node>> children;
};

class ElementNode : public Node {
public:
     ElementNode(HtmlElements, Node*);

     void setTag(HtmlElements);

     //Inherited methods.
     std::vector<std::shared_ptr<Node>> &getChildren();
     void pushChild(std::shared_ptr<Node>);
     Node* getFather() const;
};  

class TextNode : public Node {
public:
     TextNode(std::string);
     std::string getText() const;
     void setText(std::string);

     //Inherited methods.
     std::vector<std::shared_ptr<Node>> &getChildren();
     void pushChild(std::shared_ptr<Node>);
     Node* getFather() const;

private:
    std::string text;
 };

Upvotes: 0

Views: 115

Answers (2)

Fuhrmanator
Fuhrmanator

Reputation: 12882

Adding methods "as needed" will eventually be in conflict with the Interface Segregation Principle of SOLID.

Not all clients of Node will need to know all the variants, right? This means that as your base class gets bloated, clients will be forced to know about all the details. This sounds much like the Xerox Printer example that Robert Martin gives in the ISP definition.

Adding getText() to the base class means you'd violate Liskov Substitution Principle of SOLID, since some nodes would not support that method.

Your problem is how to recognize the node type when it's stored in a vector. I'd say you should re-think that part of your design, based on the Liskov Substitution Principle. Down-casting there might be a lesser of two evils.

Upvotes: 2

Solomon Slow
Solomon Slow

Reputation: 27115

The answer depends on the maturity of the design.

Early in the design, sure. It's easy. Just add the new abstract methods to the base class, and the compiler will let you know if you've forgotten to implement them in the derived classes. Piece of cake.

In a mature design and a large project, it may not be so easy. What if the new methods don't make sense for some of the derived classes? or what if they don't make sense in every place where the interface is used? I've seen it happen, and the answer to "what if..." is, you hack up workarounds for the special cases. It can take months to get everything working again, and when you're done, the system has become uglier, more brittle, less maintainable.

In a library that is used by other teams, it's a disaster. You end up breaking the builds of projects you've never even heard of. You probably end up releasing the change in a new major version, and then you find yourself having to support both the new version, and the old one because some of your customers don't want the change.

Unless you are still in the early design stage, the right answer is to define a whole new interface. The old interface existed for some reason. People still are using it for that reason. The change that you want to make is for a new reason. So, why not define a new interface that supports the new reason, and leave the old one alone? The new interface can even inherit the old one if that makes sense.

Upvotes: 2

Related Questions