Hi Vince,
Working code is always a good start
Once you get that you can then go back and re-factor to make it clean. So these comments are only meant as constructive suggestions, not trying to put the code down or anything.
First, since this is the Threads and Synchronization forum, I will begin with that. You are sleeping in your processing/background thread, not the GUI thread - this is a
good thing. You don't want to change that. I also like that you are using one of the collections in the java.util.concurrent package for sharing the snake's segments from one thread to the other - I like even more that you are using a Queue implementation. My only issue with that is that I think you are using the wrong queue: a ConcurrentLinkedQueue will probably better match your use case. The BlockingArrayQueue is really used for a produce-consumer situation where you expect one thread to be destructively consuming the Queue while another thread builds it. That doesn't really match your use case. I don't see a scenario where the difference will have an effect on your code - you protected yourself against it pretty well, but I think the intent would be clearer.
The rest of what I will address is design related. I will take them in pieces:
- There is a lot of repeated code. When you see that you should do your best to refactor it into a method, or re-arrange it so that you only need to call it once. An example is the moveSnake() method. There are a number of conditions (one for each direction), but all of the code in them is identical except the initial increment of the X or Y position. So you could change it to something like this:
That's 25ish lines of code shorter than the method you originally had, does exactly the same thing in the same way, and includes more white space and comments. Every line of code you can remove means fewer bugs.
Note that I used two comments in the method to describe what each section was doing. The fact that I could easily break the task in half like that indicates that could (should?) be made into two separate methods. But that is more your call. Short methods that do one thing makes troubleshooting and adding/changing features easier in the long run.
This method is just one example of the problem, I saw it in several places, so be sure to review the code to see where else you could consolidate the code into methods and fewer lines (if you have 2 or 3 lines of code that you repeat 2 or 3 times, consider making a method out of them, even if you have to pass several parameters in to get it correct).
- Most importantly: You have basically one super class which does everything. It is your GUI, holds your data, runs the game logic, etc... It has some inner classes that sort of break the functionality up, but you don't go far enough. Try to break your code up into tiny little pieces, where each piece can take care of itself. For example, you have a class for the Snake's XY segments, but you do a lot of work on the segments in the main class.
My suggestion would be to see if you could pull all of the methods and functionality out of the main class - make a new class which:
-- holds a collection of the snake segments
-- runs the thread which moves the snake
-- has the methods for adding and removing segments
-- has the methods for initializing the length of the snake
-- has the methods for clearing the snake (at game's end)
-- has the methods for moving the snake
-- has the methods for setting speed and length of the snake
Essentially, does
everything that the snake requires
except drawing it. Then come up with a third class which knows how to draw the snake, and has a
public void paintSnake(Graphics g, Snake s) method which your main class can delegate to.
If you do this successfully, think about doing it for all the other 'things' in your game: Food, Wall, GameBoard, Scoreboard. Each Thing has a class for all the data it needs and the methods to manage that data, and a second class which knows how to paint the Thing. Then your main class simply manages these Things (makes sure each Thing has access to the other Things it needs, that they get created when required, started, stopped, destroyed, etc...), and delegates painting to the ThingPainter classes.