Android GameWatch game for learning/review

I am hoping this is the right place. I was hoping to find someone to try and help me not only learn but have a look at what I have already done on Android.

DISCLAIMER: I am a long time (10+ years) script developer (PHP, Perl, Ruby) and am happy with OO but have not really had any experience with Java so this is a bit of an experiment so expect totally n00b questions.

I set myself a goal (it's always good to have one I find). I wanted to make an android version of the Nintendo Game & Watch game Fire. I used the graphics from the inter webs and I'm well aware that should I release this on Google Play that Nintendo will send me a nice letter telling me to remove it so I am currently just looking to use this as a learning process.

I have been reading an Android game developers book and implementing what I learn from that as well as surfing SO and the web in general for help.

So if your willing have a look at what I have so far and if you can give me some feedback/pointers I will appreciate it. I intend to update the project as often as I can with new questions that come up that I cannot solve and even write how I solved them (if I do), maybe even go so far as to use an organisation tool like Workflowy to do this.

I have pasted here the GameView file. Look in the update function where I am checking the collision of the jumper.

```
package com.example.gamewatch;

/**
* Only Currently tested on an emulator, emullating a 7" screen (Nexus 7)
* 1280 x 628
*/

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ArrayBlockingQueue;

import android.os.AsyncTask;
import android.content.Context;
import android.graphics.*;
import android.util.Log;
import android.view.MotionEvent;
import android.view.SurfaceHolder;
import android.view.SurfaceView;
import android.util.Log;

public class GameView extends SurfaceView implements SurfaceHolder.Callback {

private GameLogic mGameLogic;
private ArrayBlockingQueue inputObjectPo

Overall your code is good. If I only would say that though, you probably wouldn't learn much, and luckily for you I do have some points that you can improve:

-
Within the (currently unused) doInBackground method in ScoringTask, you are creating a Paint object that you set color and text size on, but then you're not doing anything else with it which means that it will be garbage collected directly and what you just did was useless. I know it's currently unused, but still :)

-
I don't see the meaning of this code:

try {
    Thread.sleep(16);
} catch (InterruptedException e) {
}

First of all, you should not run Thread.sleep on the UI-thread. Doing so will make your application not respond for the duration of Thread.sleep. Now, 16 ms is not a long time but it is still not good practice to make the UI-thread sleep. Right before this code snippet, you are also catching a InterruptedException - why? I really don't think that code should be able to throw such an exception. And if it does, try to avoid that (if you call Thread.sleep for example).

-
The variables mGameLogic.RUNNING and mGameLogic.FINISHED should be set to public static final where they are declared. It sounds like these are constants (RUNNING = 1 and FINISHED = 2 or similar for example), constants should be static final. Then you should instead write GameLogic.RUNNING and GameLogic.FINISHED.

-
When you iterate over the Map, you don't need to typecast entry.getValue() to AnimatedSprite. Since you are using generics correctly (Map.Entry), entry.getValue() returns an AnimatedSprite already. No need for typecasting.

-
(Minor issue, not necessarily needed either) Instead of iterating on map.entrySet() which will give you both key and value, you can iterate on map.values() which only gives you the value. (You're currently not using the key when you are iterating on the entry set)

-
In your update method, f and j are very short variable names, and therefore making their purpose unclear. firemen and jumper would be better names for those variables.

To finish the activity from the view either call getContext(), typecast that to an Activity (assuming this won't throw a ClassCastException) and call finish() on that object, or use a callback. (I would recommend using a callback, it is bad practice to let a view get it's activity. The view should be independent)

Something to read up on when you have some more experience: Should the view really be responsible for creating the GameLogic object? There is a design pattern called MVC which can be good to keep in mind (not necessarily following it to 100%) when dealing with both game logic and a view. Don't fix what isn't broken though, MVC can be heavy stuff. Learn that at your own risk.

StackExchange Code Review Q#36346, answer score: 4