سؤال

I'm writing a Window class which propagates different types of events, listed in

enum Event {WINDOW_ClOSE=0x1, WINDOW_REDRAW=0x2, MOUSE_MOVE=0x4, ...}; 

to objects which have registered for notification with the window. For each type of event, I have an abstract class which any object must extend in order to allow notification. To react to, say, a MOUSE_MOVE event, my object would inherit from MouseMoveListener, which has a process_mouse_move_event() method which is called by Window. Listening to many events can be combined by extending multiple of these classes, which all inherit from the EventListener base class. To register an object, I would call

void Window::register(EventListener* object, int EventTypes)
{
    if(EventTypes&WINDOW_CLOSE)
       /* dynamic_cast object to WindowCloseListener*, add to internal list of all
          WindowCloseListeners if the cast works, else raise error */

    if(EventTypes&MOUSE_MOVE)     
       /* dynamic_cast object to MouseMoveListener*, add to internal list of all
          MouseMoveListeners if the cast works, else raise error */

    ...
} 

This works fine, but my gripe is that EventListener is completely empty and that seems code smelly to me. I know I could avoid this by removing EventListener altogether and having a separate Window::register for each type of event, but I feel that this would blow up my interface needlessly (especially since methods other than register might crop up with the same problem). So I guess I am looking for answers that either say:

  • "You can keep doing it the way you do, because ..." or

  • "Introduce the separate Window::register methods anyway, because ..." or of course

  • "You are doing it all wrong, you should ...".

EDIT:

From the link in Igors comment: What I do above only works if there is at least one virtual member in EventListener for example a virtual destructor, so the class is not technically completely empty.

EDIT 2:

I prematurely accepted n.m.'s solution as one of the type "I'm doing it all wrong". However, it is of the second type. Even if I can call EventListener->register(Window&) polymorphically, Window needs to implement a highly redundant interface (in terms of declared methods) that allows EventListeners to register for selective notification. This is equivalent to my alternative solution described above, only with the additional introduction of the EventListener class for no good reason. In conclusion, the canonical answer seems to be:

Don't do dynamic_cast + empty base class just to avoid declaring many similar functions, it will hurt you when maintaining the code later. Write the many functions.

EDIT 3:

I found a solution (using templates) which is satisfactory for me. It does not use an empty base class any more and it does not exhibit the maintenance problem pointed out by n.m.

هل كانت مفيدة؟

المحلول 2

This is what I ended up doing:

template <int EventType> void register_(EventListener<EventType> Listener)
{
    // do stuff with Listener, using more templates
};

It turned out that static polymorphism was better suited for my needs - I just wanted to avoid writing

register_mouse_motion_event(...)
register_keyboard_event(...)

and so on. This approach also nicely eliminates the need for an empty base class.

نصائح أخرى

object->registerWindow (this, EventTypes);

Of course you need to implement registerWindow for all EventListener heirs. Let them check for event types which are relevant to them.

UPDATE

If this means you need to redesign your code, then you need to redesign your code. Why is it so? Because dynamic_cast is not a proper way to do switch-on-types. It is not a proper way because every time you add a class in your hierarchy, you need to go over and possibly update all switches-by-dynamic-cast in your old code. This becomes very messy and unmaintainable very quickly, and this is exactly the reason why virtual functions were invented.

If you do your switch-on-types with virtual functions, every time you change your hierarchy you have to do... nothing. The virtual call mechanism will take care of your changes.

مرخصة بموجب: CC-BY-SA مع الإسناد
لا تنتمي إلى StackOverflow
scroll top