I'm familiar with polymorphism in general, but I'm fairly new to C in general and templates in particular. I have to following situation with a mixture of code that I cannot change (usage of a framework, all events and templated event listeners) and code under my control (clients in the example below).
#include <string>
#include <iostream>
#include <vector>
class EventBase {
public:
virtual std::string getData() const = 0;
};
class EventA : public EventBase {
public:
std::string getData() const override {
return "Event A";
}
};
class EventB : public EventBase {
public:
std::string getData() const override {
return "Event B";
}
};
template<class T_Event>
class IEventHandler
{
public:
virtual void onEvent(const T_Event& e) = 0;
virtual void onError() = 0;
};
class ClientBase {
public:
virtual void startReceiving() = 0;
virtual void stopReceiving() {
std::cout << "ClientBase::stopReceiving" << std::endl;
}
};
class ClientA : public ClientBase, public IEventHandler<EventA> {
public:
void onEvent(const EventA& e) override {
std::cout << "ClientA::onEvent - e.getData()= " << e.getData() << std::endl;
};
void onError() override {
std::cout << "ClientA::onError" << std::endl;
};
void startReceiving() override {
std::cout << "ClientA::startReceiving" << std::endl;
};
};
class ClientB : public ClientBase, public IEventHandler<EventB> {
public:
void onEvent(const EventB& e) override {
std::cout << "ClientB::onEvent - e.getData()= " << e.getData() << std::endl;
};
void onError() override {
std::cout << "ClientB::onError" << std::endl;
};
void startReceiving() override {
std::cout << "ClientB::startReceiving" << std::endl;
};
};
int main(int, char**) {
//User Code
ClientA ca;
ClientB cb;
std::vector<ClientBase*> baseClients;
baseClients.push_back(&ca);
baseClients.push_back(&cb);
for(const auto client : baseClients){
client->startReceiving();
}
//Framework Code
EventA a;
EventB b;
std::vector<IEventHandler<EventA>*> eventHandlersA;
std::vector<IEventHandler<EventB>*> eventHandlersB;
eventHandlersA.push_back(&ca);
eventHandlersA[0]->onError();
eventHandlersA[0]->onEvent(a);
eventHandlersB.push_back(&cb);
eventHandlersB[0]->onError();
eventHandlersB[0]->onEvent(b);
//User Code
for(const auto client : baseClients){
client->stopReceiving();
}
}
See here: https://onlinegdb.com/2MYQhC2G5
What I want to do now is to have a common default implementation of onError.
To do so, I tried at least four approaches. Only the second worked. It would be nice to hear from C savants if this approach 2 is actually the way to do it.
Approach 1
Simply put onError in ClientBase and remove it from derived clients.
class ClientBase {
public:
virtual void startReceiving() = 0;
virtual void stopReceiving() {
std::cout << "ClientBase::stopReceiving" << std::endl;
}
virtual void onError(){
std::cout << "ClientBase::onError" << std::endl;
}
};
class ClientA : public ClientBase, public IEventHandler<EventA> {
public:
void onEvent(const EventA& e) override {
std::cout << "ClientA::onEvent - e.getData()= " << e.getData() << std::endl;
};
void startReceiving() override {
std::cout << "ClientA::startReceiving" << std::endl;
};
};
Fails on compile time with
error: variable type 'ClientA' is an abstract class
note: unimplemented pure virtual method 'onError' in 'ClientA'
Okay, it's abstract since it does not implement the methods needed from IEventHandler<EventA>
Approach 2
Fix the unimplemented method in ClientA but call the super class method implementation:
class ClientA : public ClientBase, public IEventHandler<EventA> {
public:
void onEvent(const EventA& e) override {
std::cout << "ClientA::onEvent - e.getData()= " << e.getData() << std::endl;
};
void onError() override {
ClientBase::onError();
};
void startReceiving() override {
std::cout << "ClientA::startReceiving" << std::endl;
};
};
Works, though under the hood I think other things are happening then originally intended (might be more of a delegation then inheritance).
Maybe mess around with templates?
Approach 3: Remove the IEventHandler from the derived clients
class ClientBase : public IEventHandler<EventBase> {
public:
virtual void startReceiving() = 0;
virtual void stopReceiving() {
std::cout << "ClientBase::stopReceiving" << std::endl;
}
virtual void onError(){
std::cout << "ClientBase::onError" << std::endl;
}
virtual void onEvent(const EventBase& e) = 0;
};
class ClientA : public ClientBase {
public:
void onEvent(const EventA& e) override {
std::cout << "ClientA::onEvent - e.getData()= " << e.getData() << std::endl;
};
void startReceiving() override {
std::cout << "ClientA::startReceiving" << std::endl;
};
};
Build system hates me:
error: non-virtual member function marked 'override' hides virtual member function
note: hidden overloaded virtual function 'ClientBase::onEvent' declared here: type mismatch at 1st parameter ('const EventBase &' vs 'const EventA &')
error: variable type 'ClientA' is an abstract class
note: unimplemented pure virtual method 'onEvent' in 'ClientA' - virtual void onEvent(const EventBase& e) = 0;
Okay, so you can override methods only if the signature matches exactly.
Approach 4: Make ClientBase templated
template<class T_Event>
class ClientBase {
public:
virtual void startReceiving() = 0;
virtual void stopReceiving() {
std::cout << "ClientBase::stopReceiving" << std::endl;
}
virtual void onError(){
std::cout << "ClientBase::onError" << std::endl;
}
virtual void onEvent(const T_Event& e) = 0;
};
class ClientA : public ClientBase<EventA> {
public:
void onEvent(const EventA& e) override {
std::cout << "ClientA::onEvent - e.getData()= " << e.getData() << std::endl;
};
void startReceiving() override {
std::cout << "ClientA::startReceiving" << std::endl;
};
};
Again, no success. This time my structures to track my clients would break:
std::vector<ClientBase*> baseClients; ----> error: use of class template 'ClientBase' requires template arguments
eventHandlersA.push_back(&ca); ---> error: no matching member function for call to 'push_back'
Do you have any more ideas on how to achieve the original goal? Or is sticking to approach 2 a good solution?
CodePudding user response：
Your insights into approaches 1-3 are generally correct:
Approach 1 failed because
ClientBasedidn't inherit fromIEventHandler<>which declared the virtual method.Approach 2 is indeed a delegation, which is fine in my opinion. Virtual methods are already a delegation - under the hood a vtable is roughly equivalent to a set of function pointers. Delegating
onErroris just one more level of indirection, and hopefully something calledonErrorisn't called frequently enough to make the performance penalty significant.Approach 3 failed because anything overriding
onEvent(const EventBase& e)needs to accept anyEventBase&, per the contract.
Approach 4 failed because ClientBase<EventA> and ClientBase<EventB> are completely different types that don't share a common base. Templates are more like type factories than types - there's no relationship between instantiations.
If you want to make the this work with inheritance, you can spell out that common base explicitly by having a non-template ClientBase and a template layer in between to implement onError:
template <typename TEvent>
class ErrorHandlingClient : public ClientBase, public IEventHandler<TEvent> {
public:
virtual void onError() override { /* ... */ }
};
class ClientA : public ErrorHandlingClient<EventA> {
public:
void onEvent(const EventA& e) override { /* ... */ }
void startReceiving() override { /* ... */ }
};
class ClientB : public ErrorHandlingClient<EventB> {
public:
void onEvent(const EventB& e) override { /* ... */ }
void startReceiving() override { /* ... */ }
};
ClientA and ClientB will have different implemenations of onError because of the template, but they can both be casted to a common ClientBase type to store in a vector.
One last opinion - if you need an abstract class to get your desired code organization, it might be a sign that your concerns aren't separated: Maybe IEventHandler<T> should really be two interfaces, or maybe error handling should be owned by some other entity.
CodePudding user response：
The basic problem is having a class template with a virtual method that does not need the template parameter. It is not wrong per se, but it can easily make one's life mighty inconvenient.
The problem with your Approach 1 is having more than one source node in the inheritance graph that has errorHandler. These functions are unrelated. Here is a simplified demo:
struct X { virtual void foo() = 0; };
struct Y { virtual void foo() {} };
struct XY : X, Y {};
XY is still abstract, despite having an implementation of foo, because there are two unrelated foos in it and the only way to unify them is to override foo in XY. This surprises a lot of people.
The best practice here (as I understand it) is moving the offending function to a common base class of X, Y and XY (create one if needed). Up the hierarchy, not down or sideways. It should be inherited virtually (not a diamond-of-death problem, since it is an ABC with no data members).
So don't do this:
template<class T_Event>
class IEventHandler
{
public:
virtual void onEvent(const T_Event& e) = 0;
virtual void onError() = 0;
};
Do this instead:
class IErrorHandler {
public:
virtual void onError() = 0;
// or whatever default implementation you want
};
template<class T_Event>
class IEventHandler : public virtual /* XXX Important! */ IErrorHandler
{
public:
virtual void onEvent(const T_Event& e) = 0;
};
class ClientBase : public virtual IErrorHandler {
virtual void onError() override {} // whatever
};
class ClientA : public ClientBase, public IEventHandler<EventA> {
virtual void onEvent(const EventA& e) {}
};
Note, the MSVC compiler may issue a warning (C4250) on this. Ignore or silence it. For your convenience, here is a collection of SO posts on this topic.