Template for creating 2D games

I am using the following template to program my 2D games in. Is there any way I can improve it?

Splash screen:

package com.dingle.template2d;

import android.app.Activity;
import android.content.Intent;
import android.os.Bundle;

public class template2d extends Activity {

@Override
public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);

    //set screen
    setContentView(R.layout.splash);

    // thread for displaying the SplashScreen
    Thread splashTread = new Thread() {
        @Override
        public void run() {
            try {
                int waited = 0;
                boolean _active;
                int _splashTime;
                 _active = true;
                 _splashTime = 500;
                while(_active && (waited < _splashTime)) {
                    sleep(100);
                    if(_active) {
                        waited += 100;
                    }
                }
                startActivity(new Intent("com.dingle.template2d.MENU"));
            } catch(InterruptedException e) {
                // do nothing
            } finally {
                finish();

            }
        }
    };
    splashTread.start();

   }

 }

Menu screen:

```
package com.dingle.template2d;

import android.app.Activity;
import android.content.Intent;
import android.os.Bundle;
import android.view.View;
import android.view.Window;
import android.widget.Button;

public class menu extends Activity {
/** Called when the activity is first created. */
@Override
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
requestWindowFeature(Window.FEATURE_NO_TITLE);
setContentView(R.layout.main);
this.button();

}

private void button(){
Button play_button = (Button)this.findViewById(R.id.play_button);
play_button.setOnClickListener(
new Button.OnClickListener() {
public void onClick(View v) {

Notions in no particular order, not probably what you're expecting though:

On the unpredictability of sleep

In the loop

int waited = 0;
while(_active && (waited < _splashTime)) {
   sleep(100);
   if(_active) {
      waited += 100;
   }
}

Method sleep might not guarantee that the time slept is actually 100 ms. It might more or it might be less depending on the context and clock accuracy etc. It might be better if you relied on the system clock instead of a counter:

long started = System.currentTimeMillis();
while(_active && System.currentTimeMillis() - start < _splashTime) {
   sleep(100);
}

If you end up using the waited counter, the if(_active) conditions might not be necessary.

On initial values

If you have no particular reason for defining the variables initial values on separate lines, you could define them in the same line

int waited = 0;
boolean _active;
int _splashTime;
_active = true;
_splashTime = 500;

becomes

int waited = 0;
boolean _active = true;
int _splashTime = 500;

On naming

I've seen underscore to denote instance variables and I after a quick skimming I thought _activeand _splashTime were such. Instead they were local variables. Perhaps not using and underscore might be more conventional.

You use underscore on instance variables as well, which is OK though I'm not a big fan of it.

There's also some inconsistency with variable names with multiple words: compare surfaceHolder and play_button. In Java it's convetional that variable names are in camel-case without spaces; playButton would be better.

Names of classes usually begin with a capital letter e.g. Template2d, Menu.

On constants

It seems to me you're trying to declare a constant _splashTime. Why not just do so with

public class template2d extends Activity {
  private static final SPLASH_TIME_IN_MILLISECONDS = 500;
}

On visibity

I would prefer if all instance variables would be either private or final.

It's at least not a bad idea to keep the scope of your variables as small as possible. In

Canvas c;
while (_run) {
    c = null;
    try {
        c = _surfaceHolder.lockCanvas(null);
        synchronized (_surfaceHolder) {
            _panel.onDraw(c);
        }
    } finally {
        // do this in a finally so that if an exception is thrown
        // during the above, we don't leave the Surface in an
        // inconsistent state
        if (c != null) {
            _surfaceHolder.unlockCanvasAndPost(c);
        }
    }
}

The canvas variable c exists outside of the while-loop while it isn't used anywhere but in it. You could declare the variable inside the loop to limit it's scope

while (_run) {
    Canvas c = null;
    try {
        c = _surfaceHolder.lockCanvas(null);
        synchronized (_surfaceHolder) {
            _panel.onDraw(c);
        }
    } finally {
        // do this in a finally so that if an exception is thrown
        // during the above, we don't leave the Surface in an
        // inconsistent state
        if (c != null) {
            _surfaceHolder.unlockCanvasAndPost(c);
        }
    }
}

On order of things

The contents of a class is usually in an order not unline the following

• The class signature

• public static constants

• private static constants

• private static variables

• private constants

• private variables

• constructors

• public methods

• private methods

It makes the code harder to follow if there are instance variable declarations in more than one place.

Lately I've been experimenting on how it feels like when everything private is tucked down to the bottom of the class and I like it. Coding conventions trump personal preferences though.

On doing just one thing

Each class and object should preferably do just one thing i.e. they should have a single responsibility. At least the class Panel contain the responsibilities of drawing the surface and responding to events. These two things could be split into two different classes if it makes sense.

On different ways of writing the same code

In this code

retry = true;
while (retry) {
    try {
        _thread.join();
        retry = false;
    } catch (InterruptedException e) {
        // we will try it again and again...
    }
}

You could achieve the same thing with a break statement to get rid of an extra variable

while (true) { // or for(;;)
    try {
        _thread.join();
        break; 
    } catch (InterruptedException e) {
        // we will try it again and again...
    }
}

or defining the logic in a method to describe your intent more clearly

while(!hasThreadStopped) {
     // Retry until thread stops
} 
...

private boolean hasThreadStopped() {
    try {
        _thread.join();
        return true;
    } catch (InterruptedException e) {
        return false;
    }
}

StackExchange Code Review Q#3838, answer score: 13