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

Question

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;
 };

Protected could be substiuted with private in this case. It is not meaningful here. I've changed it to private. — Vlad Lazar, Jul 13 '17 at 15:36
Depends a lot on the use case, I'm afraid. While I prefer the virtual function approach, readability can be greatly improved by one `dynamic_cast` done early in a long chain. Better still is bundling that chain into a single virtual function, but even that doesn't suit all cases. — user4581301, Jul 13 '17 at 15:42
You may want to consider using `template` rather than creating a bunch of node types. See also `std::list`. — Thomas Matthews, Jul 13 '17 at 15:49
This question is essentially about [heap paradox](https://en.wikipedia.org/wiki/Sorites_paradox). Yes, it is OK to add more virtual methods, but eventually class may become too cluttered so you will need to consider refactoring. — user7860670, Jul 13 '17 at 15:49
@VTT An interface is not a heap of method signatures. The heap paradox asks, "how many grains of sand constitute a _heap_ of sand?" OK, but grains of sand in that context are indistinguishable. Methods _mean_ something, and an interface has too many of them if it has two or more methods that exist for unrelated reasons. — Solomon Slow, Jul 13 '17 at 16:36
@jameslarge Such an approach falls into exactly the same trap. Whether methods are related or not is just another vague predicate. — user7860670, Jul 13 '17 at 16:54
@VTT, I won't declare an interface type, unless I've got code somewhere that uses it. That code is the reason for the interface to exist. The interface methods are related by the fact that my code uses them. Later, I may write new code that does something different to some of the same objects. I may need to define some new methods. Those new methods do not have the same relationship to the older methods because they exist for a different reason. The only relationship I care about is, do the methods exist for the same reason? or for different reasons? See my answer (below) for why I care. — Solomon Slow, Jul 13 '17 at 17:15
@Thomas Matthews, wouldn't a template restrict me to only one object class type at runtime? I don't see how that could fit in the context of a polymorphic data structure. — Vlad Lazar, Jul 13 '17 at 19:07
The `std::list` and `std::vector` use templates for applying the same algorithm when the data type changes. This removes the growth of classes derived from a node base class or an element base class. — Thomas Matthews, Jul 13 '17 at 20:54

score 2 · Answer 1 · answered Jul 13 '17 at 16:52

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.

score 2 · Accepted Answer · answered Jul 13 '17 at 20:44

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.

It turned out that I just needed to read a bit about the SOLID principles, because I was the able to refactor the code to avoid downcasting without senselessly adding new methods to the interface. Thanks for pointing me in the right direction. — Vlad Lazar, Jul 14 '17 at 13:25

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

2 Answers2