1

I am currently working on an event system for my game engine. I've thought about two ways I could implement it:

1. With inheritance:

class Event
{
    //...
    Type type; // Event type stored using an enum class
}

class KeyEvent : public Event
{
    int keyCode = 0;
    //...
}

2. With unions:

class Event
{
    struct KeyEvent
    {
        int keyCode = 0;
        //...
    }

    Type type; // Event type stored using an enum class

    union {
        KeyEvent keyEvent;
        MouseButtonEvent mouseButtonEvent;
        //...
    }
}

In both cases you have to check the Event::Type enum, to know which kind of event has occured.

When you use inheritance, you have to cast the event to get it's values (e.g. cast Event to KeyPressedEvent to get key code). So you have to cast raw pointers to the neeeded type, which is often very error prone.

When you use the union you can simply retrieve the event that has occured. But this does also mean that the size of the union in memory is always as big as it's largest possible class contained in it. This means that a lot of memory may be wasted.

Now to the question:

Are there any arguments to use one way over the other? Should I waste memory or take the risk of wrong casting?

Or is there a better solution to this which I haven't thought of?

In the end I just want to pass e.g. a KeyEvent as an Event. Then I want to check the Event::Type using the enum class. If the type is equal to say Event::Type::Key I want to get the data of the key event.

anastaciu
  • 23,467
  • 7
  • 28
  • 53
maniel34
  • 177
  • 9
  • 2
    When using inheritance, you typically use *polymorphism* to solve problems. And if you pass around object of type `Event` by value, then you can't treat it as a `KeyEvent` object because of *object slicing*. – Some programmer dude May 27 '20 at 13:57
  • @Someprogrammerdude This is what I'm trying to do: passing e.g. a `KeyEvent` as an `Event` (you already mentioned polymorphism). I can then check if it is a key event (using the enum member variable of the `Event` class) and cast the `Event*` to a `KeyEvent*`. I am just wondering if I should do this or use unions instead. (I am passing an `Event&`) – maniel34 May 27 '20 at 14:05
  • `std::variant` is superior than your union version. – Jarod42 May 27 '20 at 14:11
  • In inheritance case, you might get rid of your enum. inheritance should use polymorphism. – Jarod42 May 27 '20 at 14:13
  • @Jarod42 But I **have** to cast the `Event*` back to it's original type. And when I only pass an event reference, I can't know which it's original type was. This is why I have the enum. – maniel34 May 27 '20 at 14:22
  • 1
    The question is why do you want to know that that `Event` is a `KeyEvent`. In inheritance case, virtual method and/or visitor remove the need of (dynamic) casting. – Jarod42 May 27 '20 at 14:31
  • 1
    In `std::variant` case, dispatch can be done thanks to `std::visit`, so no casts, no additional enum. – Jarod42 May 27 '20 at 14:32
  • @Jarod42 Because `KeyEvent` has member variables that `Event` does not have (for example `int keyCode`). I have to access them. – maniel34 May 27 '20 at 14:48
  • 1
    @maniel34: I meant member is useful only if you do something with them (printing, apply action to player, ...). View like that, you have `virtual void Event::Print() const = 0` or `void Print(const Event& ev)` (the former uses native virtual dispatch, the later might use visitor pattern, or `std::visit` for `std::variant`). Either you work with generic event, or you dispatch to specific event. – Jarod42 May 27 '20 at 15:06

3 Answers3

3

Event seems more data-centric than behavior-centric, so I would choose std::variant:

using Event = std::variant<KeyEvent, MouseButtonEvent/*, ...*/>;
// or using Event = std::variant<std::unique_ptr<KeyEvent>, std::unique_ptr<MouseButtonEvent>/*, ...*/>;
// In you are concerned to the size... at the price of extra indirection+allocation

then usage might be similar to

void Print(const KeyEvent& ev)
{
    std::cout << "KeyEvent: " << ev.key << std::endl;
}
void Print(const MouseButtonEvent& ev)
{
    std::cout << "MouseButtonEvent: " << ev.button << std::endl;
}

// ...

void Print_Dispatch(const Event& ev)
{
    std::visit([](const auto& ev) { return Print(ev); }, ev);
}
anastaciu
  • 23,467
  • 7
  • 28
  • 53
Jarod42
  • 203,559
  • 14
  • 181
  • 302
1

You can use polymorphism, I believe it's a solution if your derived classes share a good amount of methods, you can have an abstract class of events, derive from it and implement these methods, you can then have them in the same container, if done correctly you won't need casting.

Something along the lines of:

Live demo

#include <vector>
#include <iostream>
#include <memory>

class Event {//abstract class
public:
    virtual void whatAmI() = 0; //pure virtual defined in derived classes
    virtual int getKey() = 0;
    virtual ~Event(){}
};

class KeyEvent : public Event {
    int keyCode = 0;
public:
    void whatAmI() override { std::cout << "I'm a KeyEvent" << std::endl; }
    int getKey() override { return keyCode; }
};

class MouseEvent : public Event {
    int cursorX = 0, cursorY = 0;
public:
    void whatAmI() override { std::cout << "I'm a MouseEvent" << std::endl; }
    int getKey() override { throw -1; }
};

int main() {
    std::vector<std::unique_ptr<Event>> events;
    events.push_back(std::make_unique<KeyEvent>()); //smart pointers
    events.push_back(std::make_unique<MouseEvent>());
    try{
        for(auto& i : events) {
            i->whatAmI();
            std::cout << i->getKey() << " Key" << std::endl;
        }
    } catch(int i){
        std::cout << "I have no keys";
    }  
}

Output:

I'm a KeyEvent
0 Key
I'm a MouseEvent
I have no keys

This is a simplified version, you would still have to deal with copy constructors, assignment operators and such.

anastaciu
  • 23,467
  • 7
  • 28
  • 53
  • 1
    @眠りネロク, oop inheritance fits exacly in situations like this, MouseEvent and KeyEvent are two of the same, each one with their dfferent way of implementing the same methods. – anastaciu May 27 '20 at 14:37
  • Where I disagree, is that `Event` seems more related to data than behavior. – Jarod42 May 27 '20 at 14:39
  • @Jarod42, I see your point, anyway discerning what is a mouseEvent and a keyEvent seems to be the main goal, as posted by the OP, and this is, I believe, a best way than using enums, it looks to me a very C-ish way to do it. – anastaciu May 27 '20 at 14:43
  • The problem is data, not functions. When I have an `int keyCode` in `KeyEvent` and an `int cursorX, cursorY` in `MouseMovedEvent`, I have to cast to get theses values properly. – maniel34 May 27 '20 at 14:46
  • @Jarod42, btw, unrelated, do you see what's wrong with [this answer](https://stackoverflow.com/a/62040149/6865932)? – anastaciu May 27 '20 at 14:46
  • @maniel34, even so I still don't see why you would casting, you can, for instance, create virtual getters and treat them properly, for instance throw something if the event isn't related to the variable you want to retrieve, Or return some type of error code. this can then be treated in the caller. – anastaciu May 27 '20 at 14:50
  • @maniel34, I added some code to illistrate my comment, anyway, Jarrod42's answer seem like a nice solution for your problem. If you don't have some shared methods it's the way to go. – anastaciu May 27 '20 at 15:02
  • @anastaciu: `int getKey() override { throw -1; }` that unconditional throw is code smell, that tends to means that the method should not be in base. – Jarod42 May 27 '20 at 15:09
  • @Jarod42, you think, what about a simple return -1? No two derived classes are the same, their bound to have different methods, this allows you to have a polymorphic container and not worry about calling methods that don't belong to the class. – anastaciu May 27 '20 at 15:13
  • 1
    @anastaciu: `std::optional` might be better, but we go for `getKeyEventData`, `getMouseEventData` which doesn't really make sense. `WhatAmI`, `UsedByUser`, `UpdateScreen` makes more sense .But as by first point was, should all those behaviors belong to `Event`? Maybe they could (I think there is at least a naming issue in that case). – Jarod42 May 27 '20 at 15:29
  • @Jarod42, yes, as things developed we can see that there is not a big relationship between the events that justify this kind of implementation, so the solution is not the most appropriate, yours is better anyway. `std::optional` seems like a good way to treat scenarios where something like this might be needed. – anastaciu May 27 '20 at 15:43
1

Events of different types typically contain very different data and have very different usage. I would say this is a bad fit for subtyping (inheritance).

Let's clear up one thing first: you don't need the type field in either case. In the case of inheritance, you can determine the subclass of you current object by casting it. When used on a pointer, dynamic_cast returns either an object of the correct subclass, or a null pointer. There is a good example on cppreference. In the case of a tagged union, you should really use a class that represents a tagged union, like std::variant, instead of having a type next to a union as members of you class.

If you were to use inheritance, the typical way to do it is to have your events in an array (vector) of Event pointers, and do things with them from the interface provided by the Event class. However, since these are events, you probably won't do the same things depending on the event type, so you will end up downcasting the current Event object into one of the subclasses (KeyEvent) and only then do stuff with it from the interface provided by the subclass (use keyCode in some way).

If however you were to use a tagged union, or variant, you can directly have your Event objects in an array, and not have to deal with pointers and downcasting. Sure, you might waste some amount of memory, but how big are your event classes anyway? Plus, you might gain performance just because the Event objects are close together in memory, in addition to not having to dereference pointers.

Again, I would say a variant is better suited to the situation than subtyping because events of different types are usually used in very different ways. For example, a KeyEvent has an associated key ("key code") among hundreds, and no position, whereas a MouseEvent has an associated button among three, maybe five but not many more, as well as a position. I can't think of a common behavior between the two, apart from very general things like a timestamp or which window receives the event.

Nelfeal
  • 12,593
  • 1
  • 20
  • 39
  • enum might be useful in inheritance in the case you want a specific level ignoring sub- derived, I meant Base A <- B <- C, `dynamic_cast(event) && !dynamic_cast(event) /* and other derived*/`. – Jarod42 May 27 '20 at 15:15
  • @Jarod42 Right, but you don't need it. I mostly agree with you, I just think cases like that should not use inheritance to begin with. Or at least I've never seen such a case where inheritance is the right thing to use. – Nelfeal May 27 '20 at 15:29
  • We agree on `std::variant` :-) – Jarod42 May 27 '20 at 15:31