HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Does my Game class have too many responsibilities?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
responsibilitiestoogamedoesmanyclasshave

Problem

I've been working on a game so as to fill out something that I can put on a CV and I was writing the main Game class. This shouldn't be a god class, but it should simply be a class that ties together the "wiring" of the game, i.e, it should start listening for input events and should start the game loop and wire any inputs to the associated handlers.

```
/**
* The main class for the Game.
* This class should start the game loop as well as display updates.
* @author Dan
*
*/
public class Game implements Disposable {
private final DisplayWindow _display;
private final Disposable _displayUpdates;
private final Scheduler _scheduler;

/**
* Create the Game.
* @param displayService - the service to use for the Display
* @param width - the initial width of the display
* @param height - the initial height of the display
*/
public Game(Scheduler gameScheduler, DisplayWindow displayWindow) {
_display = displayWindow;
_scheduler = gameScheduler;

// Wait for the display to open correctly
try {
if(!_display.isOpen().get(500, TimeUnit.MILLISECONDS)) {
System.err.println("failed to initialize OpenGL display!");
System.exit(2);
}
} catch (Exception e) {
System.err.println("OpenGL took too long to respond!");
e.printStackTrace(System.err);
System.exit(1);
}

// schedule display to update @ 60 fps
int frameInterval = (int) ((1f / displayWindow.getTargetFramesPerSecond()) * 1000f);
_displayUpdates = _scheduler.schedule(() -> displayWindow.update(), frameInterval);

}

public void startGameLoop() throws InterruptedException {
while(true) {

// Check to see if the window is closed
// Bear in mind it's OK to wait here because
// if it takes way too long we can assume that
// openGL has stopped responding

Solution

I like to trash people when their classes do too many things. I was disappointed when I saw your class because it is not so bad.

However, I don't think this class should check if the display did open. You should only create the Game once you know that the display is ready.

I'm not sure Game is an appropriate class name.

Naming member variables starting with an underscore comes from some other language; it is not standard Java.

You write too many comments. Let the code speak for itself and you won't have to worry about keeping the code and the comments in sync.

  • The comment at the beginning at the class is clearly deprecated.



  • // schedule display to update @ 60 fps might not be true.

Context

StackExchange Code Review Q#60938, answer score: 13

Revisions (0)

No revisions yet.