Overuse of static in my Java Game project?

Question

I am currently developing a little platform in Java and I wrote my own game engine for it called Bonsai. Now I'm asking myself the question "Did I overuse statics?".

On the one hand it's very convenient since I don't have to keep a reference to the game instance in each class like the map or the player. On the other hand... I have already had to strip out applet support since it was very buggy with all that static stuff in there.

So my question is, since you may be much more experienced Java programmers than I, should I get rid of all the statics? And if yes, what would be an effective approach to get to something like this:

public void draw(Graphics2D) {
  if (this.game.time() > this.timer) {
    this.game.image.draw(this.tiles[this.game.animation.get("tileAnim")], x, y, null)
  }
}

instead of:

public void draw(Graphics2D) {
  if (Game.time() > this.timer) {
    Image.draw(this.tiles[Animation.get("tileAnim")], x, y, null)
  }
}

or even worse in the map editor:

   public void control() {
     if(this.map.game.input.keyPressed(...)) {
       this.map.game.sound.play(...);
     }
   }

EDIT
Based on the answers I decided to have an GameObject class which provides wrapper methods for each component. Map, player etc. then subclass from it, this way I all the this.game calls are hidden behind the scens an it still looks nice on the frontside:

public class GameObject {
    private Game game;

    public GameObject(Game g) {
     game = g;
    }

    public Game Game() {
     return game;
    }

    public GameAnimation Animation() {
     return game.animation;
    }

    public GameInput Font() {
     return game.input;
    }

    // ...

    public long Time() {
     return game.time();
    }
}

Now the code looks like this:

public class Player() {
    public Player(Game g, int xpos, int ypos) {
      super(g);
      // do other stuff
    }

    public void jump() {
      // jump code
      Sound().play("jump");
    }
}

Or is this even worse Java?

EDIT2
Ok I'd already run into problems using method calls the compiler is giving me errors since it can't find methods of my subclassed Game in the original one, i think I'm jsut gonna use plain fields here.

EDIT3
Ok my GameObject class now looks like this, everything works fine again and I can reimplement that applet support :)

public class GameObject {
    protected Game game;
    protected GameAnimation animation;
    protected GameFont font;
    protected GameInput input;
    protected GameImage image;
    protected GameSound sound;

    public GameObject(Game g) {
     game = g;
     animation = game.animation;
     font = game.font;
     input = game.input;
     image = game.image;
     sound = game.sound;
    }
    }

Answer 1

static exists as a semantic tool like any other. When something is static, that has meaning to what the class is trying to model. In your case, you're effectively using static to create global storage and you've apparently run into some of the problems the prompt the general rule of globals being "bad."

If the behavior of your static interfaces is forcing your hand on design issues and implementation issues in ways you don't want it to (like your applet support, e.g.), then yes, it's a problem and you should consider refactoring into a more appropriate design.

Should you get rid of all the statics? Maybe. If it's impossible for more than a single instance of a particular object to exist at any one time, and you're willing to handle the cost of synchronizing access to the object, then maybe something can remain static. I don't see anything in your example that I would absolutely keep static based on my guess of what they do, though.

(Regarding your apparent distaste for the chained object references, you may want to look into the Law of Demeter and techniques that can bring code more in line with it. I don't think code should adhere to it strictly, but there should generally be an attempt to keep the chained method calls somewhat short.)

Answer 2

In my view, using statics to share some global state instead of having to inject it into client classes makes code more difficult to test, reuse, hack, etc, because you are coupling those client classes to the class whose static members they are using.

Here's a contrived example:

What if, say, you come up with 2 different types of Game class and you want to benchmark them both.

Or maybe you want to extend your engine to run two completely different types of game (maybe a platformer and a shoot-em-up, who knows) and it just so happens you can do this by modifying Game and keeping all the other classes the same. (I said it was contrived.)

With all your other classes accessing static members of the Game class, you have to write 2 versions of your application, each with its own implementation of Game.

If you simply passed a Game object into every class that needs it, you can subclass Game and pass any sort of Game into the client classes that need it.

Answer 3

Excessive usage of statics usually hardens making changes to code and decreases its testability.

Consider replacing static objects with normal objects. You may pass them into constructor of your object. This way, they would be easily replaces with another implementation (real or mock/stub for testing). This technique is called dependency injection (DI).

E.g., you've passed to your app three objects:

• gameTimer (extracted from Game class to make it less God-like)
• image
• animation

and saved it to fields. This way instead of

public void draw(Graphics2D) {
  if (Game.time() > this.timer) {
    Image.draw(this.tiles[Animation.get("tileAnim")], x, y, null)
  }
}

you'll have

public void draw(Graphics2D) {
  if (this.gameTimer.time() > this.timer) {
    this.image.draw(this.tiles[this.animation.get("tileAnim")], x, y, null)
  }
}

The difference looks subtle, but actually is significant, because your code will become more modular.

• Each object has single responsibility and thus can be tested well.
• If you'll implement another version of Animation class (Animation2), you'll need to make changes in your main() function only, not in the Animation class.
• Single glance on your code shows which objects are used, no need to look for static methods calls.

Answer 4

My personal experience with statics is really two-fold. Firstly, I find that statics are an anti-pattern that often obfuscate and hide the fact that I have a problem with my design. If I find myself needing a static variable, I have to ask myself why. However, there are times when a static is needed, and in fact appropriate. In these cases, I attempt to isolate the pieces that are truly global to the program, and place them in their own singleton class, rather than a static variable somewhere. Although it may just be semantics, making them a singleton keeps things a little cleaner in an OO sense in my experience.

Answer 5

First of all.

You don't have to get rid of all your static code just because that would make it better on "paper".

You really have to understand what the difference is between instance code (non-static) and class code (static)

static code (class methods/attributes) is used when the methods/attributes do no need an instance of the class to work. A very good example is the image draw method: Image.draw()

instance methods/attributes are useful to keep state of a given object which must be kept separate from data in other objects.

For instance, if you have Player class in your game and you have two instances player1 and player2 it makes sense each of them to have their own score:

 public class Player {
     private int score;
     private String name;
     etc.....
 }

 Player one = new Player("Player 1");
 display( one.score );

 Player two = new Player("Player 2");
 display( two.score );

Instead of having to create artifacts to keep each player score (like putting them in arrays where each index is an attribute and make that array static etc. etc.)

Secondly

You may reduce the constructs you mentioned object1.atr2.other.next.etc by assigning to objects appropriate attributes and performing encapsulation in the right way.

If a object b needs to access the N'th element of another a it is likely that said attribute belongs to the object b instead of a or probably that object a should provide a method to avoid exposing it's internals.

It even makes the code easier to read:

ie.

public void draw(Graphics2D) {
  if( this.game.needsDrawing() ) {
       this.game.draw();
  }
}

instead of:

public void draw(Graphics2D) {
    if (this.game.time() > this.timer) {
        this.game.image.draw(this.tiles[this.game.animation.get("tileAnim")], x, y, null)
    }
}

Again, it depends on the situation, there might be scenarios where you don't need an instance (again, like the draw() utility method of Image)

Finally.

Instance methods allow you to use polymorphism while class methods do not (at least, in Java and other statically typed languages).

So, you may benefit from using runtime delegation and polymorphism if your code is instance code. Take for instance the State pattern you can't use it if all your code is static, but you can with instance code:

class Game {
     GameState state = GameState.getInitialState( this );

     void run() {
         while( state.alive ) {
              do X Y Z
              state.updateState();
          }
     }
  }


class GameState {
    Game context;
    static GameState getInitialState( Game g ) {
        return new StartGameState(g);
    }
    void updateState();
}

class StartGameState {
     void updateState() {
         if( this.context.someCondition() ) {
             this.context.state = new MidGameState();
         }
     }
 }


class MidGameState {
     void updateState() {
         if( this.context.someOtherCondition() ) {
             this.context.state = new EndGameState();
         }
     }
 }

class EndGameState {
     void updateState() {
        Game over... 
     }
 }

And once again, only if this make sense in terms of object orientation, like does the object have attributes whose data is required? If not it may be good to keep that section of code static.

All these concepts (encapsulation, polymorphism, abstraction, inheritance, etc) are the very nature of OO technology and covered in OOA/D while they may seem like syntactic sugar (and most of the times they are) your experience will tell you when you should have something as class code and when as instance code.