patternjavaMinor
Simplified version of 2048 game
Viewed 0 times
versionsimplifiedgame2048
Problem
I tried to get back into Java by implementing a simplified version of the 2048 game. What I am mainly looking for is ways to reduce code duplication, all of my move functions have a very similar but distinct pattern.
Suggestions on better patterns, practices or optimizations are welcome, keeping in mind that I am trying to implement my functionality without relying on any Libraries (built-in or otherwise).
```
public class Game {
int[][] board;
int COLNUM = 4;
int ROWNUM = 4;
int globalPointer = 0;
// demonstration code to illustrate usage
public static void main (String[] args) {
Game game = new Game();
game.printBoard();
game.moveSouth();
game.printBoard();
game.moveWest();
game.printBoard();
game.moveWest();
game.printBoard();
game.moveEast();
game.printBoard();
game.moveNorth();
game.printBoard();
}
// simplified constructor, could add option to provide 2D array to instantiate the board
public Game() {
board = new int[ROWNUM][COLNUM];
board[0][0] = 2;
board[1][0] = 8;
board[2][0] = 4;
board[3][0] = 8;
board[0][1] = 1;
board[0][2] = 2;
board[0][3] = 1;
board[2][1] = 4;
board[2][2] = 4;
board[2][3] = 4;
}
public void printBoard() {
String output = "";
for (int i = 0; i = 0; row--) {
// if not zero we try to suck in the next tile
if (board[row][col] != 0) {
// if we had a prior zero tile then shift the column
if (globalPointer >= row) {
shiftRowTiles(row, col, true);
}
}
}
}
}
public void moveWest() {
for (int row = 0; row = 0; col--) {
// if not zero we try to suck in the next tile
if (board[row][col] != 0) {
Suggestions on better patterns, practices or optimizations are welcome, keeping in mind that I am trying to implement my functionality without relying on any Libraries (built-in or otherwise).
```
public class Game {
int[][] board;
int COLNUM = 4;
int ROWNUM = 4;
int globalPointer = 0;
// demonstration code to illustrate usage
public static void main (String[] args) {
Game game = new Game();
game.printBoard();
game.moveSouth();
game.printBoard();
game.moveWest();
game.printBoard();
game.moveWest();
game.printBoard();
game.moveEast();
game.printBoard();
game.moveNorth();
game.printBoard();
}
// simplified constructor, could add option to provide 2D array to instantiate the board
public Game() {
board = new int[ROWNUM][COLNUM];
board[0][0] = 2;
board[1][0] = 8;
board[2][0] = 4;
board[3][0] = 8;
board[0][1] = 1;
board[0][2] = 2;
board[0][3] = 1;
board[2][1] = 4;
board[2][2] = 4;
board[2][3] = 4;
}
public void printBoard() {
String output = "";
for (int i = 0; i = 0; row--) {
// if not zero we try to suck in the next tile
if (board[row][col] != 0) {
// if we had a prior zero tile then shift the column
if (globalPointer >= row) {
shiftRowTiles(row, col, true);
}
}
}
}
}
public void moveWest() {
for (int row = 0; row = 0; col--) {
// if not zero we try to suck in the next tile
if (board[row][col] != 0) {
Solution
General findings
Java Naming Conventions
Please follow Java Naming Conventions. Your identifiers
Force immutability
The content of your member variable
Code duplication
As you found yourself your program has lots of duplicated code.
To be able to reduce this we have to find similar code fragments and refactor them to become identical code fragments. Of cause these refactoring would be less risky if we had UnitTests that would gard the desired behavior...
Disclaimer: None of the following code ist tested and may neither be compilable nor working as expected. This only demonstrates the technique to reduce code duplication.
However, This kind of refactoring is usually done my introducing new local variables:
Lets look at your code:
We change it so that is uses the same variable names in the loops:
At this point we see that there is also differing behavior. Unfortunately the differeing behavior is in the control instructions. But it is much easier to convert the code to be "the same" when the behavior difference is in "calculations". So lets try to make that change:
Now we can encapsulate the differing behavior into new objects by Providing a new interface :
Now you can copy this to all other
```
private void moveTo(Direction direction){
for (int outerIndex = 0; outerIndex < direction.getOuterLoopMax(); outerIndex++) {
globalPointer = globalPointerInitialValue;
for (int innerIndex = 0; innerIndex < direction.getInnerLoopMax(); innerIndex++) {
// if
Java Naming Conventions
Please follow Java Naming Conventions. Your identifiers
COLNUM an ROWNUM imply that they are constants but you did not declare them neither static nor final.Force immutability
The content of your member variable
board does not change during the lifetime of your program (after initialisation). Therefore it sould be declared final.Code duplication
As you found yourself your program has lots of duplicated code.
To be able to reduce this we have to find similar code fragments and refactor them to become identical code fragments. Of cause these refactoring would be less risky if we had UnitTests that would gard the desired behavior...
Disclaimer: None of the following code ist tested and may neither be compilable nor working as expected. This only demonstrates the technique to reduce code duplication.
However, This kind of refactoring is usually done my introducing new local variables:
Lets look at your code:
public void moveNorth() {
for (int col = 0; col = 0; col--) {
// if not zero we try to suck in the next tile
if (board[row][col] != 0) {
// if we had a prior zero tile then shift the row
if (globalPointer >= col) {
shiftColTiles(row, col, true);
}
}
}
}
}We change it so that is uses the same variable names in the loops:
public void moveNorth() {
int outerLoopMax = COLNUM;
int innerLoopMax = ROWNUM;
boolean isReverse = false;
int globalPointerInitialValue = 0;
for (int outerIndex = 0; outerIndex = 0; innerIndex--) {
// if not zero we try to suck in the next tile
if (board[outerIndex][innerIndex] != 0) {
// if we had a prior zero tile then shift the outerIndex
if (globalPointer >= innerIndex) {
shiftColTiles(outerIndex, innerIndex, isReverse);
}
}
}
}
}At this point we see that there is also differing behavior. Unfortunately the differeing behavior is in the control instructions. But it is much easier to convert the code to be "the same" when the behavior difference is in "calculations". So lets try to make that change:
public void moveEast() {
int outerLoopMax = ROWNUM;
int innerLoopMax = COLNUM;
boolean isReverse = true;
int globalPointerInitialValue = COLNUM - 1;
}Now we can encapsulate the differing behavior into new objects by Providing a new interface :
interface Direction{
int getGlobalPointerInitValue();
int getOuterLoopMax();
int getInnerLoopMax();
boolean isNotEdge(int globalPointer, int innerIndex);
boolean isReverse();
int getRow(int outerIndex);
int getColumn(int innerIndex);
}
// ...
public void moveEast() {
Direction direction = new Direction(){
@Override
public int getGlobalPointerInitValue(){
return COLNUM - 1;
}
@Override
public int getOuterLoopMax(){
return ROWNUM;
}
@Override
public int getInnerLoopMax(){
return COLNUM;
}
@Override
public boolean isNotEdge(int globalPointer, int innerIndex){
return globalPointer >= innerIndex;
}
@Override
public boolean isReverse(){
return true;
}
@Override
public int getRow(int outerIndex){
return outerIndex;
}
@Override
public int getColumn(int innerIndex){
return getGlobalPointerInitialValue()-innerIndex;
}
}
for (int outerIndex = 0; outerIndex < direction.getOuterLoopMax(); outerIndex++) {
globalPointer = globalPointerInitialValue;
for (int innerIndex = 0; innerIndex < direction.getInnerLoopMax(); innerIndex++) {
// if not zero we try to suck in the next tile
if (board[direction.getRow(outerIndex)][direction.getColumn(innerIndex)] != 0) {
// if we had a prior zero tile then shift the outerIndex
if (direction.isNotEdge(globalPointer,innerIndex)) {
shiftColTiles(direction.getRow(outerIndex), direction.getColumn(innerIndex), direction.isReverse());
}
}
}
}
}Now you can copy this to all other
move methods and adjust the return values of the anonymous classes of type Direction. This way you have differing code incorporating the different behavior and identical code incormorating the identical behavior in all move methods which you can extract to a parameterized method:```
private void moveTo(Direction direction){
for (int outerIndex = 0; outerIndex < direction.getOuterLoopMax(); outerIndex++) {
globalPointer = globalPointerInitialValue;
for (int innerIndex = 0; innerIndex < direction.getInnerLoopMax(); innerIndex++) {
// if
Code Snippets
public void moveNorth() {
for (int col = 0; col < COLNUM; col++) {
globalPointer = 0;
for (int row = 0; row < ROWNUM; row++) {
// if not zero we try to suck in the next tile
if (board[row][col] != 0) {
// if we had a prior zero tile then shift the column
if (globalPointer <= row) {
shiftRowTiles(row, col, false);
}
}
}
}
}
public void moveEast() {
for (int row = 0; row < ROWNUM; row++) {
globalPointer = COLNUM - 1;
for (int col = COLNUM - 1; col >= 0; col--) {
// if not zero we try to suck in the next tile
if (board[row][col] != 0) {
// if we had a prior zero tile then shift the row
if (globalPointer >= col) {
shiftColTiles(row, col, true);
}
}
}
}
}public void moveNorth() {
int outerLoopMax = COLNUM;
int innerLoopMax = ROWNUM;
boolean isReverse = false;
int globalPointerInitialValue = 0;
for (int outerIndex = 0; outerIndex < outerLoopMax; outerIndex++) {
globalPointer = globalPointerInitialValue;
for (int innerIndex = 0; innerIndex < innerLoopMax; innerIndex++) {
// if not zero we try to suck in the next tile
if (board[innerIndex][outerIndex] != 0) {
// if we had a prior zero tile then shift the column
if (globalPointer <= innerIndex) {
shiftRowTiles(innerIndex, outerIndex, isReverse);
}
}
}
}
}
public void moveEast() {
int outerLoopMax = ROWNUM;
int innerLoopMax = COLNUM;
boolean isReverse = true;
int globalPointerInitialValue = COLNUM - 1;
for (int outerIndex = 0; outerIndex < outerLoopMax; outerIndex++) {
globalPointer = globalPointerInitialValue;
for (int innerIndex = innerLoopMax - 1; innerIndex >= 0; innerIndex--) {
// if not zero we try to suck in the next tile
if (board[outerIndex][innerIndex] != 0) {
// if we had a prior zero tile then shift the outerIndex
if (globalPointer >= innerIndex) {
shiftColTiles(outerIndex, innerIndex, isReverse);
}
}
}
}
}public void moveEast() {
int outerLoopMax = ROWNUM;
int innerLoopMax = COLNUM;
boolean isReverse = true;
int globalPointerInitialValue = COLNUM - 1;
}interface Direction{
int getGlobalPointerInitValue();
int getOuterLoopMax();
int getInnerLoopMax();
boolean isNotEdge(int globalPointer, int innerIndex);
boolean isReverse();
int getRow(int outerIndex);
int getColumn(int innerIndex);
}
// ...
public void moveEast() {
Direction direction = new Direction(){
@Override
public int getGlobalPointerInitValue(){
return COLNUM - 1;
}
@Override
public int getOuterLoopMax(){
return ROWNUM;
}
@Override
public int getInnerLoopMax(){
return COLNUM;
}
@Override
public boolean isNotEdge(int globalPointer, int innerIndex){
return globalPointer >= innerIndex;
}
@Override
public boolean isReverse(){
return true;
}
@Override
public int getRow(int outerIndex){
return outerIndex;
}
@Override
public int getColumn(int innerIndex){
return getGlobalPointerInitialValue()-innerIndex;
}
}
for (int outerIndex = 0; outerIndex < direction.getOuterLoopMax(); outerIndex++) {
globalPointer = globalPointerInitialValue;
for (int innerIndex = 0; innerIndex < direction.getInnerLoopMax(); innerIndex++) {
// if not zero we try to suck in the next tile
if (board[direction.getRow(outerIndex)][direction.getColumn(innerIndex)] != 0) {
// if we had a prior zero tile then shift the outerIndex
if (direction.isNotEdge(globalPointer,innerIndex)) {
shiftColTiles(direction.getRow(outerIndex), direction.getColumn(innerIndex), direction.isReverse());
}
}
}
}
}private void moveTo(Direction direction){
for (int outerIndex = 0; outerIndex < direction.getOuterLoopMax(); outerIndex++) {
globalPointer = globalPointerInitialValue;
for (int innerIndex = 0; innerIndex < direction.getInnerLoopMax(); innerIndex++) {
// if not zero we try to suck in the next tile
if (board[direction.getRow(outerIndex)][direction.getColumn(innerIndex)] != 0) {
// if we had a prior zero tile then shift the outerIndex
if (direction.isNotEdge(globalPointer,innerIndex)) {
shiftColTiles(direction.getRow(outerIndex), direction.getColumn(innerIndex), direction.isReverse());
}
}
}
}
}Context
StackExchange Code Review Q#160254, answer score: 4
Revisions (0)
No revisions yet.