Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am creating a game that contains a Board and Pieces. The Board is basically a 2D array of Pieces. [Think smaller chess board]

One of the things that I want to accomplish is to be able to retrieve the left piece of a given.. (Also want to be able to retrieve the right, up, and down).

The easiest way to achieve this is to simply do something like:

board[i-1][j] //which will give me the piece that's to the left of board[i][j]

The problem with just doing this is that this can get ugly and there is no error checking (array out of range).

I have a feeling that there may be a better data structure to use than a 2D array.

Also, I have two implementations that work, but neither really seem like a good idea

I have the following (untested) Java classes, which is a small scale of my application:

the class Board:

public class Board 
{
protected Piece[][] board;

public Board()
{
    board = new Piece[7][7];
    int ctr = 0;
    for(int i = 0; i < 7; i++)
    {
        for(int j = 0; j < 7; j++)
        {
            board[i][j] = new Piece(i, j, ctr++);
        }
    }
}

public Piece PieceAt(int i, int j)
{
    return board[i][j];
}

public Piece LeftOf(Piece p)
{
    return PieceAt(p.verticalIndex, p.horizontalIndex - 1);
}
}

And the Piece class

public class Piece 
{
    int verticalIndex;
    int horizontalIndex;
    int value;

    public Piece(int i, int j, int value)
    {
        this.horizontalIndex = i;
        this.verticalIndex = j;
        this.value = value;
    }

    public Piece GetLeftPiece(Board b)
    {       
        return (this.horizontalIndex == 0 ? null : b.PieceAt(this.horizontalIndex, this.verticalIndex - 1));
    }
}

The first implementation sits in Board.java. The problem with this is that If I want to call it, it's a big ugly: b.LeftOf(b.PieceAt(3,3)) where b is of type Board.

The second implementation seems like a better idea, however since the Pieces no nothing about each other, I have to include board in the method signature.

share|improve this question

1 Answer 1

up vote 1 down vote accepted

The first location of the leftOf method is the way to go. Putting it in Piece isn't very cohesive. The latter would be just as ugly or uglier (b.pieceAt(3,3).getLeftPiece(b)). The syntax board.leftOf(b.pieceAt(3,3)), read aloud, communicates roughly your intent but might read better as board.getPieceLeftOf(b.pieceAt(3,3)). Further, the best way is to remove the middle man and change the interface:

public Piece getPieceLeftOf(int x, int y) {
    int leftLocation = x - 1;
    // Validate that location
    return pieceAt(leftLocation, y);
}

//  ...later
board.getPieceLeftOf(3, 3);

Some style notes/thoughts:

  • Since this is written in Java, you should be following the standard naming conventions. camelCase for method names
  • You have a lot of package and package-private fields, it's a better practice to make them private and then expose the ones that actually need to be exposed outside of the class. (Effective Java Item 13).
  • If there is no error checking for the pieceAt or leftOf methods then add it and communicate through javadocs what happens if there is no left piece, a request is made out of bounds, etc. Those are both implemenation and game-type rules that need to be considered.
  • It seems like a simple 2D array is fine for this application. That is a pretty common structure for 2D chess/checkers type games (quick access, easy mental model, etc).
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.