8

I have one base class which holds a map for function pointers like this

typedef void (BaseClass::*event_t)();
class BaseClass {
    protected:
        std::map<std::string, event_t> events;
    public:
        // Example event
        void onFoo() {
            // can be added easily to the map
        }
};

Handling this works prefect, but now i want to make BaseClass an abstract base class to derive from like this:

 class SpecificClass : public BaseClass {
     public:
         void onBar() {
             // this is gonna be difficult!
         }
 };

Although i can access the map from SpecificClass i am not able to add onBar because the event_t type is only defined for the BaseClass! Is there any possibility (maybe with templates?) which does not lead to define the event_t for each class i will use...

(It is not neccessary to use templates! Any good/suitable approach would be nice.)

More background information:

This whole thing is for a text based RPG. My base class could be called Location and the specifc one any location e.g. CivicCenter. Each Location object subscribes to my EventSystem which notifies all neccessary objects when i fire an event. Therefore i want to store in a map some pointers to private functions holding the actions with their "name" like onSetOnFire (xD) as the key.

Christian Ivicevic
  • 10,071
  • 7
  • 39
  • 74

4 Answers4

2

You have to use static_cast:

event_t evt = static_cast<event_t>(&SpecificClass::onBar);

This is because it is slightly dangerous to cast to event_t, you could accidently apply it to a BaseClass instance.

How it works (for the skeptical):

class BaseClass {
public:
    typedef void (BaseClass::*callback_t)(); // callback method
    void doSomething(callback_t callback) {
        // some code
        this->*callback();
        // more code
    }
    void baseCallback(); // an example callback
};

class DerivedClass : public BaseClass {
public:
    void derivedCallback();
    void doWhatever() {
        // some code
        doSomething(&BaseClass::baseCallback);
        // more code
        doSomething(static_cast<callback_t>(&DerivedClass::derivedCallback));
        // et cetera
};

Here is what you should avoid, and why this is potentially dangerous:

void badCodeThatYouShouldNeverWrite()
{
    BaseClass x;
    // DO NOT DO THIS IT IS BAD
    x.doSomething(static_cast<callback_t>(&DerivedClass::derivedCallback));
}

The requirement for a static_cast makes it so you can't "accidentally" pass DerivedClass method pointers in. And if you think this is dangerous, just remember that it's a pointer, and pointers are always dangerous. Of course, there are ways you can do this that involve creating helper classes, but that requires a lot of extra code (possibly making a class for every function you want to pass as a callback). Or you could use closures in C++11, or something from Boost, but I realize that a lot of us do not have that option.

Dietrich Epp
  • 205,541
  • 37
  • 345
  • 415
  • I'm pretty sure that's not legal C++. I'm not certain, but I don't think you can cast member pointers like that, nor can you later call them through the base class, unless the member is _in_ the base class. – Nicol Bolas Sep 27 '11 at 19:20
  • Even I think, it's not legal C++. – Nawaz Sep 27 '11 at 19:24
  • @Nawaz: It works. I tried it. It compiles. I used it in a program. – Dietrich Epp Sep 27 '11 at 19:27
  • @DietrichEpp: I still feel it's not legal. – Nawaz Sep 27 '11 at 19:28
  • @Nawaz: Your feelings are irrelevant. This is part of the C++ language. When I say "I used it in a program" I am not talking about a test program, but rather a finished application which was delivered. – Dietrich Epp Sep 27 '11 at 19:30
  • 1
    @DietrichEpp: Just because something compiled and ran does not mean that it "works". It could mean that you got away with something on that particular compiler. – Nicol Bolas Sep 27 '11 at 19:32
  • 1
    @DietrichEpp: Just because it works, doesn't mean *"This is part of the C++ language"* (as you claimed). Quote from the spec to support your claim. (As for your it-works-argument, even this works: http://ideone.com/nLrPU .. but it clearly is undefined behaviour according to the spec). – Nawaz Sep 27 '11 at 19:33
  • This really works... however @Nawaz was right! It seems somehow dirty. – Christian Ivicevic Sep 27 '11 at 19:35
  • 1
    @Nawaz: I quote from the relevent specs pretty often on this site, but it's a lot of work and I don't feel the need to do it for something as mundane as using `static_cast` on a member function pointers. You can get this stuff out of any old C++ textbook (well, at least the two I looked at). Of course it's dangerous, it's a POINTER, and of course there are probably better ways to do it in C++11 or otherwise. – Dietrich Epp Sep 27 '11 at 19:45
  • @Nawaz: If you still feel this is nonstandard, feel free to quote the standard yourself. You only have to quote one or two lines to show something is wrong, but you have to practically quote the whole thing to show a program is correct. – Dietrich Epp Sep 27 '11 at 19:47
  • I have found a solution by myself - have a look at my own answer! – Christian Ivicevic Sep 28 '11 at 14:45
2

This can't be done with your current map as it stands. Think about what would happen if you could put a child method into the map. Then you could pull a pointer-to-child-member (masquerading as base) out of the map, call it on a base class instance pointer, and then how would it call a derived class on a base class instance which obviously couldn't work.

Would a polymorphic approach work?

Mark B
  • 95,107
  • 10
  • 109
  • 188
2

Yes; stop using member pointers.

The more correct way of doing what you want is to have an event type and an object pointer. So an event fires on a specific object. The event type would be a non-member function (or a static member). It would be passed the object pointer. And it would call some actual member function of that object.

Nowadays, the event type could be a std/boost::function. However, since the function parameters have to stay the same type for all events, this doesn't really fix your problem. You can't call SpecificClass::onBar from a BaseClass pointer unless you do a cast to a SpecificClass. And the event calling function would not know to do this. So you still can't put SpecificClass::onBar in the std/boost::function object; you still need some standalone function to do the cast for you.

This all just seems to be a terrible use of polymorphism. Why does SpecificClass need to derive from BaseClass at all? Can't they just be two unrelated classes?

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • Please have a look at my first post where i entered more details. – Christian Ivicevic Sep 27 '11 at 19:29
  • @ChristianIvicevic: That still doesn't explain why `CivicCenter` needs to be a derived class at all. Nor does it change the truth of anything I said. – Nicol Bolas Sep 27 '11 at 19:31
  • The `Location` class should hold the map with the pointers and it stores the `notify` function derived from another interface which applies to every `Location` object which is the reason why i want to derive rather than manually copy&paste these parts. (for generalization) – Christian Ivicevic Sep 27 '11 at 19:34
  • @ChristianIvicevic: My point is that derive or copy&paste are not your only options. – Nicol Bolas Sep 27 '11 at 19:46
  • Seems i misunderstood your thoughts. May you show me some sample code? – Christian Ivicevic Sep 27 '11 at 19:49
  • @ChristianIvicevic: Sample code for how to fix your problem (as I described above) or sample code for how to not use polymorphism? I can't give you the latter because you didn't really say why you made `CivicCenter` a derived class, rather than just a particular location. What changes about `CivicCenter`'s interface or data in the deriving? – Nicol Bolas Sep 27 '11 at 19:56
  • Specific locations should provide different actions which are not just global ones (they will part of `Location`) eg. `onCheckStats`. A `CivicCenter` should (hardcoded for now; later dynamically setup via XML) provide e.g. `onMountebank`. – Christian Ivicevic Sep 27 '11 at 20:02
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/3839/discussion-between-nicol-bolas-and-christian-ivicevic) – Nicol Bolas Sep 27 '11 at 20:12
  • I have found a solution by myself - have a look at my own answer! – Christian Ivicevic Sep 28 '11 at 14:45
2

After some thought and a redesign i was able to achieve what i wanted. Although i am stubborn and still using inheritance i have reimplemented the map. This is how it works now:

class Location {
    // ...

    protected:
        std::map<std::string, std::function<void(void)>> m_mEvents;
};

And now i can handle it like this:

class CivicCenter : public Location {
    public:
        CivicCenter() {
            // this is done by a macro which lookes better than this
            this->m_mEvents["onTriggerSomething"] =
                  std::bind(&CivicCenter::onTriggerSomething, this);
        }

        void onTriggerSomething() {
            // ...
        }

    // ...
};

With easy use of std::bind i am able to implement generic function pointers. When using parameters like in std::function<void(int, int)> remeber to use either boost's _1 and _2 or lambda expressions like me:

std::function<void(int,int)> f = [=](int a, int b) {
    this->anotherFunctionWithParams(a, b);
};

But this is just pointed out due to completeness of my solution.

Christian Ivicevic
  • 10,071
  • 7
  • 39
  • 74