views:

129

answers:

6

I'm part of a team designing the server for a client/server model naval warfare video game (University course). We have a fairly concrete (well, I think) system design, but there is one aspect that bothers me.

The basic layout of the system is:

Server [thread] (handles incoming connections)
|
Game [thread] (deals with game events in it's work queue and sending messages to clients)
|--Environment (holds environment variables, deals with collision)
|--Client(s) [thread] (handles incoming messages from client sockets and adds events to the Game's work queue)
    |--Ship (holds game data, eg: hit points, fire power, etc)
    |--Parser (parses client messages and creates game event objects)
|--GameEvent (event objects held in a queue that can preform appropriate work and send appropriate responses to clients)

Now, my issue is that both Client and GameEvent (and probably Environment, once we get to it) need a reference to the Game object that they belong to.

Clients need to add GameEvent's to the Game's work queue.

GameEvent's need to access other game data (other Client's Ships, Environment).

Is there a better/more conventional method instead of having these objects store a local reference to their Game? What about declaring all of Game's methods as static? We only need to handle one game at a time, so there wouldn't ever be more than one instance of Game...

I'm sure there is a convention for a system with one central object that has many helper objects that need to reference it.

+1  A: 

If there's really only logically ever one instance, you can use a singleton. The canonical form of a singleton is:

public enum Singleton {
    INSTANCE;

    // fields and methods here
}

That way, you don't have to shoehorn everything into static methods (though, if you want to write static methods that reference INSTANCE, that's fine too). Any code that wants to access the singleton just uses Singleton.INSTANCE, and you don't have to pass it around if you don't want to.

Chris Jester-Young
Carefull ... Singleton is considered by many to be an antipattern : http://www.google.com/search?q=singleton+antipattern
Guillaume
@Guillaume: Indeed, it's best not to be too gung-ho about using singletons, though there's a time and place for them (function objects without state being my current favourite example). Knowing when your situation is such a case or not is the million-dollar question.
Chris Jester-Young
+1  A: 

Passing the reference around will keep your options open, it can still be a reference to an actual static object. Also the concept of a request context might be useful, an object that holds all references needed to process a single request, and you pass that around.

fhj
+1  A: 

Check out Inversion of Control (IOC) and containers.

That way, in your Client and GameEvent classes, whenever you need access to the Game, you just do something like:

var game = IoC.Resolve<Game>(); 

And then use the game instance methods...

GalacticJello
I'm wondering what the overhead there is. Is Resolve<Game> fast enough to be called millions of times per second or is it going to bog things down?
Zan Lynx
You don't call resolve more than once in a class where you need it...
GalacticJello
This is Java, not C#. You don't get niceties like `IoC.Resolve<Game>()`; at the very best, you can use `IoC.resolve(Game.class)`. :-) (Java has erasure-based generics: the type parameters will not be available at runtime.)
Chris Jester-Young
Yea, I knew the C# would incite some feelings, but for pseudo code, it's better than trying to explain beans and spring. :) I still think IOC is the way to go in this instance. Especially if his Environment class constructor currently isn't passed a Game reference, but may need one in the future. Using IOC, you just have to change the code in the constructor to instantiate a game bean, rather than change the constructor signature, which may break other areas...
GalacticJello
I agree that IoC/DI is a good way for this instance (my post about singletons notwithstanding...some would see it as the devil's advocate option), though, I actually liked Landei's answer better, because it touches on current technology in this area (Google Guice).
Chris Jester-Young
+1  A: 

There's nothing wrong with passing references to a constructor and storing them. You should also introduce an interface that will mediate access between your Game and your client and environment objects.

CurtainDog
I'm not sure exactly what you mean, but if I understand, that was the point of using a queue for game event's. What exactly do you mean by an interface to mediate access?
Eric Coutu
+1  A: 

I would strongly advise not using a singleton or a static class in your design. There are lots of reasons for this, but the one that will probably affect you most immediately is that it makes things very hard to test. At testing time, there will probably be more than one instance of Game.

A common convention to ameliorate having one big central object with lots of helper objects is to try to avoid one big central object. You note that there are different clients of 'Game' with different needs. Maybe your interface on Game is too wide.

Burleigh Bear
I don't think so. What it comes down to is that I need some object to deal with client requests (messages, eg: changeSpeed). Given the threaded approach, we decided a queue would make sense to avoid concurrency issues. All our Game object will ever do is ask GameEvent's in it's queue to preform their work and send an appropriate response message to clients. Game is not really "big," it's just that somewhere, there is going to have to be a root node that all objects of the game can use to reference other objects of the game. Sounds like passing references around is the better option though.
Eric Coutu
Sorry, just a note on the above: Game doesn't handle client sockets itself, just GameEvent objects in it's queue that have been parsed and offered by Client objects.
Eric Coutu
+3  A: 

Did you consider using a Dependency Injection framework like Guice? There you have config classes called "modules", where you bind your interface Game to an implementation (you can decide if you want a singleton or new instances). A Client class would look like

public class Client {
   private final Game game;

   @Inject
   public Client(Game game) {
      this.game = game; 
   }   

   ... 
}

You can construct this class as usual, providing a Game instance (e.g. for testing, using a mock Game class). But if you let Guice create this instance for you (which doesn't need to be directly, it works as well if another class injects Client), you get automatically the instance specified in your Guice module.

I know it takes some time to wrap your head around that concept, but I can confirm that this leads to cleaner, more flexible code.

Landei
+1 Dependency injection is a good idea, though you should call it that, not just DI. The OP is still a student, and many students have probably not heard of things like dependency injection and inversion of control. :-P
Chris Jester-Young
Changed. Thanks!
Landei