Skip to main content
GameDev.net gamedev.net
🔒 Locked

Is it okay to use a reference instead of a pointer in this case?

Started by Drakkcon Jan 23, 2008 at 2:25 PM 20 replies 5.9k views
Original Post
Drakkcon
Drakkcon
This is something I have never tried before. My game engine is based around events. The main event class of my event system is the EventDispatcher. The EventDispatcher is responsible for making sure all the event subscribers get notified when an event is raised. It has a list of all subscribers, for each event type. I use this data structure:

typedef std::list <EventHandler&> EventHandlerList;
typedef std::map  <std::string, EventHandlerList> EventHandlerMap;
It's important that the EventHandlerMap in the EventDispatcher be up do date, so I must use handles to refer to my instance of EventDispatcher (which is usually contained in the Game or Application class). My point is, I can't just have each class who uses to EventDispatcher to raise events have a copy, they must have some kind of handle. Originally I was using boost::shared_ptr. Every class that raised events would have a boost::shared_ptr . Then I realized that this wouldn't work because my EventDispatcher is allocated on the stack, but shared_ptr is used to make sure that heap memory gets deallocated, so there'd probably be some runtime crash or assert or something. So then I decided to use naked pointers, that is, EventDispatcher*. This would work fine, but since there should never be a null EventDispatcher and it should never accidentally be deleted, I thought that I could maybe give each class an EventDispatcher& instead, like this:

class VideoSystem {
private:
  const EventDispatcher& dispatcher;

public:
  VideoSystem(const EventDispatcher& dispatcher) : dispatcher(dispatcher) { 
    dispatcher.RaiseEvent(VideoSystemCreatedEvent());
  }
};

class Game {
private:
  VideoSystem video;
  EventDispatcher dispatcher;

public:
  Game() : video(dispatcher) {
   // whatever
  } 
};
So my question is this: Is this okay? I have never seen this before, but I see no reason why it's a bad idea. On the contrary, it seems very elegant but I want to be sure before I commit to this design. Also, if it's not a bad design, why is it so uncommon?
ToohrVyk
ToohrVyk
It's fine.

EDIT: I am referring only to the VideoSystem example, not the std::list thing above it.

[Edited by - ToohrVyk on January 23, 2008 3:28:50 PM]
Drakkcon
Drakkcon
Great, that clears it up. Now I just wonder why I never see this technique. It's not even mentioned in the C++ faq for instance.
mossmoss
mossmoss
Offhand, that sounds like a **very, very bad idea**. STL containers work via copy mechanics, which is fine for pointers and objects (with decent copy constructors).

References, however, do not. When the container attempts to perform certain operations on the "values" in the container, and do a copy, you are going to get very unexpected behaviour... your will begin losing some data and duplicating other data.

Go with raw pointers and assert on NULL pointers, if there should not be any.


stonemetal
stonemetal
Probably because it is considered a poor programming practice to hand out your privates. Normally you would see it in reverse. Where things register with the event dispatcher which would then pole them for events and push them out to listeners.
mossmoss
mossmoss
I'd actually be surprised if it compiled... A good STL implementation would prevent you from making such a mistake.
SiCrane
SiCrane
It might actually work for std::list. It would probably die horrible painful deaths on any other container. Even if it does compile for std::list, you'd be in undefined behavior land.
Drakkcon
Drakkcon
Okay, so I won't use std::list but what about a class having a private reference to the dispatcher that it uses to raise events?

Also: It definitely compiles and I'm using Visual C++ 8
mossmoss
mossmoss
Quote:
Original post by Drakkcon
Okay, so I won't use std::list but what about a class having a private reference to the dispatcher that it uses to raise events?


Sure, that sounds normal.

Quote:

Also: It definitely compiles and I'm using Visual C++ 8


MSVC is not a paragon of compiler technology. That it compiles doesn't make it a good idea.
SiCrane
SiCrane
Things you stick in a standard container need to be CopyConstructible and Assignable. References are CopyConstructible but not Assignable. Similarly, classes with reference members generally don't satisfy Assignable. About the only time they do is when the reference is to a fixed member variable (in which case it's generally pointless).
Drakkcon
Drakkcon
Quote:
Original post by stonemetal
Probably because it is considered a poor programming practice to hand out your privates. Normally you would see it in reverse. Where things register with the event dispatcher which would then pole them for events and push them out to listeners.


What privates are being handed out? The handler's interface has only a single function for handling events. Pass a handler to dispatcher and whenever a listener wants to raise an event it tells the dispatcher to send the event to the proper handlers.

My problem is that I need the listeners to know which dispatcher is responsible for raising the event, since dispatcher is not a singleton. Thus, they need some kind of pointer. I tried using references but I have never seen that before.

Forget about the std::list, my actual design does not use that. Sorry for miscommunicating.
Drakkcon
Drakkcon
The class VideoSystem:
class VideoSystem {private:  const EventDispatcher& dispatcher;public:  VideoSystem(const EventDispatcher& dispatcher) : dispatcher(dispatcher) {     dispatcher.RaiseEvent(VideoSystemCreatedEvent());  }};


Will not be stored in an STL container.
Drakkcon
Drakkcon
Also, thanks for your help everyone, specifically SiCrane, ToohrVyk, and mossmoss. Sorry again for the miscommunication that confused everyone into thinking that I have a list of references stored in an STL container.
stonemetal
stonemetal
class Game {
private:
VideoSystem video;
EventDispatcher dispatcher;

^^^ is a private.
Game() : video(dispatcher) {
^^^^
that is being handed out to any thing that wishes to use the event dispatcher, or am I confused?

I would put a super class for video that had the post event function and an internal queue of events make event dispatcher have a friend function that pulled events out of the queue then processed them.
Drakkcon
Drakkcon
Quote:

that is being handed out to any thing that wishes to use the event dispatcher, or am I confused?


No, it's only being handed out to that specific VideoSystem instance.
Zahlman
Zahlman
Quote:
Original post by stonemetal
class Game {
private:
VideoSystem video;
EventDispatcher dispatcher;

^^^ is a private.
Game() : video(dispatcher) {
^^^^
that is being handed out to any thing that wishes to use the event dispatcher, or am I confused?

I would put a super class for video that had the post event function and an internal queue of events make event dispatcher have a friend function that pulled events out of the queue then processed them.


This is fine: the dependency is inverted. We would have a design problem if the VideoSystem *took* the EventDispatcher from game (e.g. by an accessor).

Quote:
Original post by Drakkcon
I realized that this wouldn't work because my EventDispatcher is allocated on the stack


Er, how exactly is that? Do you just create a local Game instance in main() or something?
Drakkcon
Drakkcon
Quote:

Er, how exactly is that? Do you just create a local Game instance in main() or something?


Yes, actually:
int main(int, char**) {    Game game;    game.Run();     return 0;}
Zahlman
Zahlman
Good :) You may want to flesh that out a little (e.g. stuff argv into a vector and pass it to the constructor; catch exceptions and report errors). But yeah, there's no especially good reason I can think of not to have the Game class give references to its subsystems to each other. It's certainly nicer than working with Singletons. :)

(EDIT: Careful of order of initialization! And if you have a cyclic dependency, you'll have to use pointers, because references must be initialized, and there's a chicken-and-egg problem with the initialization.)

But you should consider, for each pair of subsystems, if they really need to talk to each other like that. Consider, for example: instead of talking to a SoundManager, construct a Sound object. The Sound object can register itself with a Manager if needed (or simply play with static data of the class), but the calling code needn't know that Manager (or static data) exists. This is how we get object-oriented ;)
Drakkcon
Drakkcon
Quote:

(EDIT: Careful of order of initialization! And if you have a cyclic dependency, you'll have to use pointers, because references must be initialized, and there's a chicken-and-egg problem with the initialization.)


Ooo, I didn't even think of that. Thanks for warning me, I'll try to avoid that. It seems similar to the circular reference problem with shared_ptrs (except dealing with initialization and not freeing).

Quote:

But you should consider, for each pair of subsystems, if they really need to talk to each other like that. Consider, for example: instead of talking to a SoundManager, construct a Sound object. The Sound object can register itself with a Manager if needed (or simply play with static data of the class), but the calling code needn't know that Manager (or static data) exists. This is how we get object-oriented ;)

This is a flaw in my current project right now. The way I originally envisioned it, I would create the individual parts of my engine like middleware (they don't talk to each other), and then create an event system. The components would not be aware of the event system so I would wrap them with manager classes that would expose methods allowing me to create, destroy, and perform special actions with objects, referenced with a string and using std::map. These managers would raise events whenever you did any of these things.

This seems like a pretty clunky system now that I look at it (especially the name-based lookup). I'm not really sure how I would do such a thing now. I guess you'd have to make each class event-aware, but then how would you tie in 3rd-party libraries to your engine? I was reading the book "Game Coding Complete" and Mike McShaffrey recommends strongly that you do event based programming, but I really don't see what the benefits are.

Anyway thanks for all your advice on my code, and also advice you've given to me in the past. Do you have a website for all the coding you do? They say "don't read source code" but I get the feeling that reading yours would be pretty educational =)

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.