Chess game project

I'm currently just working on a Chess game project to improve my object-oriented skills. It is currently only at the starting stages and there is quite a bit missing (such as error checking) but I thought before I go any further I should get some advice on if I am using correct practices and if there's anything I could be doing better.

Game.java

public class Game {

    public static void main(String [] args)
    {

        //Creates players and board
        Player p1 = new Player();
        Player p2 = new Player();
        Board board = new Board();

        board.displayBoard();

        //Return players command coordinates (current and target)
        Coord[] command = p1.getCommand();

        //Gets piece from supplied coordinates
        Piece piece = board.getPiece(command[0]);

        if (piece != null && piece.getColor() == p1.getColor())
        {
            board.movePiece(piece, command[1]);

        }

        board.displayBoard();
    }
}

Player.java

```
import java.util.Scanner;

public class Player {

private boolean white;
private boolean turn;
static boolean whiteChosen = false;

public Player()
{
if (whiteChosen)
white = turn = false;
else
white = turn = whiteChosen = true;
}

public Coord[] getCommand()
{
Scanner input = new Scanner(System.in);
char[] command;

do
{
System.out.print("Move: ");
command=input.nextLine().toCharArray();
System.out.println();
} while (!validFormat(command));

input.close();

int x1, x2, y1, y2;
Coord pos1, pos2;
x1 = (int) command[0] - 97;
x2 = (int) command[3] - 97;
y1 = (int) (command[1] - '0' - 8) * -1;
y2 = (int) (command[4] - '0' - 8) * -1;
pos1 = new Coord(x1, y1);
pos2 = new Coord(x2, y2);

return new Coord[] {pos1, pos2};

}

boolean validFormat(char[] command)

OOP

OOP wise your code is pretty good.

I would change the piece type (and probably the color as well) from strings/boolean to enums, as they are easier to handle.

I would also separate the Board model and logic from the actual displaying of the board (you can leave your current method though, rename it to toString, and use it for debugging purposes), the advantage will be that you can easily switch how you present the board if you separate these concerns (ideally, you create interfaces). I would also extract the code to get player input from the model of the player (again, with interfaces), so that you can easily switch how you get input.

Your Piece class is a little odd, in that it accepts int[] position in the constructor, and then Coord pos in the setter for the same field. Also, it is the only place that your setter returns the object itself.

Naming

• shortening variable names generally hurts readability. DIMENSION isn't too much longer than DIM, but easier to read. Same goes for Coord and pos.

• your Player fields are somewhat confusing. What do white, turn and whiteChosen mean? The naming doesn't make this all that clear.

Comments

Most of your comments did not tell me anything I did not already know. //Initializes a piece. for example is not very helpful.

For method level comments, it is ok to duplicate the information the code already gave me, but you should use JavaDoc style comments, and you should tell me a bit more about corner cases. For example, movePiece says: //Moves piece to given coordinates, but it doesn't tell me what happens if I pass a piece that doesn't exist, or a coordinate outside the field, etc. Another example: getPiece tells me that it Returns piece at given coordinates, but not eg when it returns null, what that means, etc.

Misc

• in Java, it is customary to place curly brackets on the same line as the opening statements.

• use curly brackets even for one line statements for improved readability and to avoid future bugs.

• you should not have magic numbers in your code; extract these to static variables at class level. That way, it is clear which numbers actually belong together, it's easier to change, and the name will tell a reader what the number means (eg why 97?)

StackExchange Code Review Q#82232, answer score: 4