Reputation: 475
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
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();
};
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
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