patternjavaMinor

2048 Java clone

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

javaclone2048

Problem

I'm making a 2048 clone and my method to move and merge tiles is really long. I've heard of helper methods but I'm a little lost as to how I should use them. I feel like my code is a little too long and can be made shorter but I don't know how. What can I do to make it shorter and cleaner? I'll also take any constructive criticism on anything else part of the method.

```
public boolean move(Direction direction)
{
boolean didMove = false;
int multiplier = 2;
if (direction == Direction.RIGHT) {

//int i represents rows; int j represents columns
for (int i = 0; i slideTile = new ArrayList<>();

//Copying row w/o 0s to ArrayList
for (int j = 0; j 0; j--) {
if (slideTile.get(j).equals(slideTile.get(j-1))) {
slideTile.set(j, multiplier*slideTile.get(j-1));
score = score + multiplier*slideTile.get(j-1);
slideTile.set(j-1, 0);
}
}

//Removing 0s that may have resulted from merging
for (int j = 0; j slideTile = new ArrayList<>();

//Copying row w/o 0s to ArrayList
for (int j = 0; j slideTile = new ArrayList<>();

//Copying row w/o 0s to ArrayList
for (int i = 0; i slideTile = new ArrayList<>();

//Copying row w/o 0s to ArrayList
for (int i = 0; i 0; i--) {
if (slideTile.get(i).equals(slideTile.get(i-1))) {
slideTile.set(i, multiplier*slideTile.get(i-1));
score = score + multiplier*slideTile.get(i-1);
slideTile.set(i-1, 0);
}
}

//Removing 0s that may have resulted from merging
for (int i = 0; i <slideTile.size(); i++ ) {
if (slideTile.get(i) == 0) {
slideTile.remove(i);
}
}

//Adding 0s to end of ArrayList to slide tiles up
int size = slideTile.size();
for (int i = 0; i < (GRID_SIZE - size); i++){
slideTile.add(0, 0);
}

//Copying ArrayList row back to game grid.
for (int i = 0; i < GRID_SIZE; i++) {
this.grid[i][j] = slideTile.get(i);
}
}

Solution

Just some bad practices:

Indentation

Your indentation is a mess. It should look like this:

public boolean move(Direction direction) {
    boolean didMove = false;
    int multiplier = 2;
    if (direction == Direction.RIGHT) {

        // int i represents rows; int j represents columns
        for (int i = 0; i  slideTile = new ArrayList<>();

            // Copying row w/o 0s to ArrayList
            for (int j = 0; j  0; j--) {
                if (slideTile.get(j).equals(slideTile.get(j - 1))) {
                    slideTile.set(j, multiplier * slideTile.get(j - 1));
                    score = score + multiplier * slideTile.get(j - 1);
                    slideTile.set(j - 1, 0);
                }
            }

            // Removing 0s that may have resulted from merging
            for (int j = 0; j  slideTile = new ArrayList<>();

            // Copying row w/o 0s to ArrayList
            for (int j = 0; j  slideTile = new ArrayList<>();

            // Copying row w/o 0s to ArrayList
            for (int i = 0; i  slideTile = new ArrayList<>();

            // Copying row w/o 0s to ArrayList
            for (int i = 0; i  0; i--) {
                if (slideTile.get(i).equals(slideTile.get(i - 1))) {
                    slideTile.set(i, multiplier * slideTile.get(i - 1));
                    score = score + multiplier * slideTile.get(i - 1);
                    slideTile.set(i - 1, 0);
                }
            }

            // Removing 0s that may have resulted from merging
            for (int i = 0; i < slideTile.size(); i++) {
                if (slideTile.get(i) == 0) {
                    slideTile.remove(i);
                }
            }

            // Adding 0s to end of ArrayList to slide tiles up
            int size = slideTile.size();
            for (int i = 0; i < (GRID_SIZE - size); i++) {
                slideTile.add(0, 0);
            }

            // Copying ArrayList row back to game grid.
            for (int i = 0; i < GRID_SIZE; i++) {
                this.grid[i][j] = slideTile.get(i);
            }
        }
        didMove = true;
    }
    return didMove;
}

Other

-
ArrayList slideTile = new ArrayList<>();

Declaring with the interface type is better, unless you specifically want a method in the ArrayList. Also, don't use ArrayList unless you know the size of the List, and you know it will stay like so. Since ArrayList uses an array for storage, when you add something to the List that exceeds its size, it will create a new array and move everything into that one. Also, when you remove something, ArrayList has to move all the objects after the removed one 1 space back.

But now to the point. Use a LinkedList instead.

-
Some of your lines are tightly spaced. Give them more space:

slideTile.set(j, multiplier*slideTile.get(j-1)); to:

slideTile.set(j, multiplier * slideTile.get(j - 1));

-
Much of your code is duplicated. Try merging them together and put this in the place where it is different:

direction == Direction.RIGHT ? / if it is Direction.RIGHT, otherwise: / : (direction == Direction.LEFT ? / if it is Direction.LEFT, otherwise: / : (direction == Direction.UP ? / if it is Direction.UP, otherwise: / : / It's Direction.DOWN. Do what you have to do./))

(I know it looks horrible, but it is MUCH shorter than doing the almost same thing 4 times)

Replace the comments with what you have to do, or:

int temp;
if(direction == Direction.RIGHT) {
    // temp = whatever;
} else if(direction == Direction.LEFT) {
    // temp = whatever;
} else if(direction == Direction.UP) {
    // temp = whatever;
} else {
    // It's Direction.DOWN
    // temp = whatever;
}

Code Snippets

public boolean move(Direction direction) {
    boolean didMove = false;
    int multiplier = 2;
    if (direction == Direction.RIGHT) {

        // int i represents rows; int j represents columns
        for (int i = 0; i < GRID_SIZE; i++) {
            ArrayList<Integer> slideTile = new ArrayList<>();

            // Copying row w/o 0s to ArrayList
            for (int j = 0; j < GRID_SIZE; j++) {
                if (grid[i][j] != 0) {
                    slideTile.add(grid[i][j]);
                }
            }

            // Merging tiles that are next to each other if
            // they are equal and updating score.
            for (int j = slideTile.size() - 1; j > 0; j--) {
                if (slideTile.get(j).equals(slideTile.get(j - 1))) {
                    slideTile.set(j, multiplier * slideTile.get(j - 1));
                    score = score + multiplier * slideTile.get(j - 1);
                    slideTile.set(j - 1, 0);
                }
            }

            // Removing 0s that may have resulted from merging
            for (int j = 0; j < slideTile.size(); j++) {
                if (slideTile.get(j) == 0) {
                    slideTile.remove(j);
                }
            }

            // Adding 0s to beginning of ArrayList to slide tiles right
            int size = slideTile.size();
            for (int j = 0; j < (GRID_SIZE - size); j++) {
                slideTile.add(0, 0);
            }

            // Copying ArrayList row back to game grid.
            for (int j = 0; j < GRID_SIZE; j++) {
                this.grid[i][j] = slideTile.get(j);
            }
        }
        didMove = true;
    }

    if (direction == Direction.LEFT) {

        // int i represents rows; int j represents columns
        for (int i = 0; i < GRID_SIZE; i++) {
            List<Integer> slideTile = new ArrayList<>();

            // Copying row w/o 0s to ArrayList
            for (int j = 0; j < GRID_SIZE; j++) {
                if (grid[i][j] != 0) {
                    slideTile.add(grid[i][j]);
                }
            }

            // Merging tiles that are next to each other if
            // they are equal and updating score.
            for (int j = 0; j < slideTile.size() - 1; j++) {
                if (slideTile.get(j).equals(slideTile.get(j + 1))) {
                    slideTile.set(j, multiplier * slideTile.get(j + 1));
                    score = score + multiplier * slideTile.get(j + 1);
                    slideTile.set(j + 1, 0);
                }
            }

            // Removing 0s that may have resulted from merging
            for (int j = 0; j < slideTile.size(); j++) {
                if (slideTile.get(j) == 0) {
                    slideTile.remove(j);
                }
            }

            // Adding 0s to end of ArrayList to slide tiles left
            int size = slideTile.size();
            for (int j = 0; j < (GRID_SIZE - size); j++) {
                slideTile.add(0);
            }

int temp;
if(direction == Direction.RIGHT) {
    // temp = whatever;
} else if(direction == Direction.LEFT) {
    // temp = whatever;
} else if(direction == Direction.UP) {
    // temp = whatever;
} else {
    // It's Direction.DOWN
    // temp = whatever;
}

Context

StackExchange Code Review Q#79424, answer score: 6

Revisions (0)

No revisions yet.