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

2048 game clone in Java

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

Problem

I made a 2048 game clone in Java for training, and I am trying to make it more efficient.

The problem I found is the paintComponent() method always resets all the graphics before doing anything, so I am forced to repaint all 4 rows of the game every time I want to repaint. Obviously I didn't want to override paint() and/or the update() method because I don't know how to correctly override them and it's not a good idea anyway.

The game works so far, but, before I proceed further, I am looking for ideas on how to only repaint the rows that get affected. For example, if the player presses ← and only the bottom two rows get affected and the top ones don't change at all, I only want to redraw the bottom rows for efficiency. I had this idea of having an array of 4 int elements where each element represents a row. If the is_moved flag becomes true I set that row in that array to equal 0. If that row didn't change, the index in the array for that row will be -1. But still, because of paintComponent() resetting everything beforehand, I have to every time I want to repaint(). I have to redraw all rows, even if some of them don't need to be repainted.

```
public class Game_Panel extends JPanel implements KeyListener
{
//instance variables
public TILE panel[][];
public byte current_tiles;
public boolean achieved_goal;

public static final int default_start_A = 2;
public static final int default_start_B = 4;
public static final int HW = 489;
public static final int seperation_length = 4;
public static final int block_width = 119;
public static final int block_center = 119>>1;
public static final int RANDOM = 101;
public static final byte ROWS_COLS = 4;
public static final byte real_end = ROWS_COLS-1;
public static final byte fake_end = 0;
public static final byte left_increment = 1;
public static final byte right_increment = -1;

//keyboard ascii numbers
public static final byte LEF

Solution

Since Simon mentioned that re-writing the output is off-topic here (plus that's not really my area of expertise), I'll just do a general code review.

And FWIW: For a simple project like this, you don't need to do such an optimization.

public class Game_Panel extends JPanel implements KeyListener


Don't put everything into one big class. Use an MVC pattern you should have (at least):

  • a class for the game logic completely independent from IO and Swing (Model)



  • an extension of JPanel for the output (View)



  • a class which basically represents the window/application and which handles raw input and lets the first two classes communicate (Controller with a bit of View)



  • possibly a class just consisting of the main function



public static final int default_start_A = 2;
public static final int default_start_B = 4;

public static final Color TWO = new Color(238,228,218);
public static final Color FOUR = new Color(237,224,200);
public static final Color EIGHT = new Color(242,177,121);


Any time you find yourself numbering/lettering variables, or creating any other similar sequence of variables, you are doing something wrong. These should be arrays or lists of some kind.

All constants (static final variables) should have names consisting only of capital letters.

public static final int block_center = 119>>1;


It's completely unnecessary to use bit-shifting for multiplication/division by two here.

public static final byte real_end = ROWS_COLS-1;
public static final byte fake_end = 0;


I don't get the meaning of these to variable names.

//same as generate method, but thought it'd be a waste
//to call it when we're initializing.


What the? It what way would that be a "waste"? One of the top principles of good programming is DRY (Don't Repeat Yourself). There is absolutely no reason for this.

Random row_col = new Random();


Don't repeatedly create a Random object each time you need a random number. Create a single one at the start of the program and use that.

(That's all for now. There are still much more problems. Maybe I'll come back later.)

Code Snippets

public class Game_Panel extends JPanel implements KeyListener
public static final int default_start_A = 2;
public static final int default_start_B = 4;

public static final Color TWO = new Color(238,228,218);
public static final Color FOUR = new Color(237,224,200);
public static final Color EIGHT = new Color(242,177,121);
public static final int block_center = 119>>1;
public static final byte real_end = ROWS_COLS-1;
public static final byte fake_end = 0;
//same as generate method, but thought it'd be a waste
//to call it when we're initializing.

Context

StackExchange Code Review Q#51012, answer score: 7

Revisions (0)

No revisions yet.