Simplified version of 2048 game

Question

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 < ROWNUM; i++) {
            String row = "";
            for (int j = 0; j < COLNUM; j++) {
                row += board[i][j] + ",";
            }
            row += "\n";
            output += row;
        }
        System.out.println(output);
    }

    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 moveSouth() {
        for (int col = 0; col < COLNUM; col++) {
            globalPointer = ROWNUM - 1;
            for (int row = ROWNUM -1; row >= 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 < ROWNUM; row++) {
            globalPointer = 0;
            for (int col = 0; col < COLNUM; 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, 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);
                    } 
                }
            }
        }
    }

    private void shiftRowTiles(int currentRow, int currentCol, boolean reverse) {
        // if we have same tiles we suck them in
           if (board[globalPointer][currentCol] == 0 || board[globalPointer][currentCol] == board[currentRow][currentCol]) {
               if (currentRow > globalPointer || (reverse && (globalPointer > currentRow))) {
                   board[globalPointer][currentCol] += board[currentRow][currentCol];
                   board[currentRow][currentCol] = 0;
               }
           } else {
               if (reverse) {
                   globalPointer--;
               } else {
                   globalPointer++;
               }
               // look around to shift away zeroes in sequences
               shiftRowTiles(currentRow, currentCol, reverse);
           }
       }

    private void shiftColTiles(int currentRow, int currentCol, boolean reverse) {
        // if we have same tiles we suck them in
           if (board[currentRow][globalPointer] == 0 || board[currentRow][globalPointer] == board[currentRow][currentCol]) {
               if (currentCol > globalPointer || (reverse && (globalPointer > currentCol))) {
                   board[currentRow][globalPointer] += board[currentRow][currentCol];
                   board[currentRow][currentCol] = 0;
               }
           } else {
               if (reverse) {
                   globalPointer--;
               } else {
                   globalPointer++;
               }
               // look around to shift away zeroes in sequences
               shiftColTiles(currentRow, currentCol, reverse);
           }
       }
}

Answer 1

General findings

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 < 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);
                } 
            }
        }
    }
}    

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 < 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);
                } 
            }
        }
    }
} 

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 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());
                } 
            }
        }
    }
}

The instances of the Direction interface could be moved to be constants of an enum (or concrete classes if you prefer them...) that could be placed in its own file:

enum MoveDirection implements Direction{
  EAST{
    @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;
    }
  },
  NORTH{
     //
  },
  WEST{
     //
  },
  SOUTH{
     //
  }      
}

After this your code would simplify to:

public void moveEast() {
   moveTo(MoveDirection.EAST);
}
public void moveNorth() {
   moveTo(MoveDirection.NORTH);
}
//...

and then you could even omit the "specialized" move methods:

public static void main (String[] args) {
    Game game = new Game();
    game.printBoard();
    game.moveTo(MoveDirection.SOUTH);
    game.printBoard();
    game.moveTo(MoveDirection.WEST);
    game.printBoard();
    game.moveTo(MoveDirection.WEST);
    game.printBoard();
    game.moveTo(MoveDirection.EAST);
    game.printBoard();
    game.moveTo(MoveDirection.NORTH);
    game.printBoard();
}