A Coder
A Coder

Reputation: 475

Virtual function's parameters not used in all subclasses; any better way to design?

I'm writing a few related C++ classes, and I'm having some trouble with the design of a particular inherited function.

Specifically, the classes are all "operations" on trees, and I need to be able to execute a set of arbitrary tree operations on a tree. I have a function named ExecuteOperation() in each class.

The problem is that in some of the classes, I need more supplementary information than in others. Right now, I simply pass the extra info to all of the classes, with the abstract base class defined like this:

class BasicOperation  {
    public:
    //...
        virtual void ExecuteOperation(Tree* tree, multimap<int,Foo*> extra1, multimap<int,Foo*> extra2) = 0;
    //...
}

Descendants of a BasicOperation subclass, "SpecialOperation", need the parameters extra1 and extra2, while descendants of "NonspecialOperation" don't use these parameters at all.

The caller of ExecuteOperation is typically something like this:

Tree* tree;
std::vector<BasicOperation*> operations;
//...
for(size_t i=0;i<operations.size();i++)  {
    operations[i]->ExecuteOperation(tree,extra1,extra2);
}

The filling of extra1 and extra2 depends on the particular tree and may be computation intensive (best not to recreate them inside each ExecuteOperation call).

Is there any better way to design this so that only SpecialOperation objects get the parameters passed to them? Or is it generally just fine to have unused parameters passed to classes like the NonspecialOperation's?

It feels weird to define NonspecialOperation::ExecuteOperation() with two parameters it doesn't use.

The only thing I've thought of so far is to give the SpecialOperation objects each the pointer back to the caller, and store the extra information in the caller object. I don't really like this solution because extra1 and extra2 are too dependent on the current state; parallelization issues aside, that setup just "feels" wrong.

Also, wrapping tree, extra1, and extra2 in another structure to pass to ExecuteOperation might make it look cleaner, and makes sense because extra1 and extra2 are extra descriptors for the tree, but I don't know if that's the best solution:

struct TreeEx  {
    Tree* tree;
    multimap<int,Foo*> extra1, extra2;
};
//in BasicOperation:
virtual void ExecuteOperation(TreeEx* tree_with_info) = 0;

Upvotes: 4

Views: 1126

Answers (2)

SigTerm
SigTerm

Reputation: 26429

Use

virtual void ExecuteOperation(Tree* tree);

Without extra arguments.

Pass extra arguments via constructor in classes derived from BasicOperation. Add methods to the tree that would allow BasicOperation to extract required data.

Also see Command pattern and Visitor pattern

--EDIT--

The extra arguments are particular to the tree

Make a virtual function in base Tree class that creates command you need

class Tree{
public:
    virtual BasicOperation* makeSpecificTreeOperation() = 0;
    virtual ~Tree();
};
  • for every type of command.

Override it in children

 class DerivedTree: public Tree{
    ...
    virtual BasicOperation* makeSpecificTreeOperation();
    ...
 };

 class DerivedCommand: public BasicOperation{
 ....
 public:
    DerivedCommand(ExtraTreeData& extraData);
 };

 BasicOperation* DerivedTree::maksSpecificTreeOperation(){
     return new DerivedCommand(this->extraData);
 }

and return required command class, passing data into command using constructor.

See Abstract Factory pattern and Builder pattern.

Upvotes: 3

Seb
Seb

Reputation: 2715

It's clean to have a reference back to the caller if it is via an interface. Thus, you invert the dependency. You'll have :

virtual void ExecuteOperation(ICaller* caller)

Then, you can either implement it in the actual caller (but you apparently have concurrency issues in your context) or implement it in a dedicated class. You reduce the coupling that way, which is a good thing. Of course, ICaller provides getters for extra1 and extra2 which will be invoked only for the relevant BasicOperation implementations.

Upvotes: 1

Related Questions