As I was writing my first part of my Chess question, I realized that I couldn't fit all my code into one question (I exceeded the character limit), so I will continue here.The pieces all share the same structure, and I had to have a way of storing them together, so I created the Piece interface:import java.util.List;import javax.swing.Icon;public interface Piece {    /**     * Returns the possible moves for the piece, with the given board and position.     *      * @param board The board.     * @param pos The position in which the piece is located in the board.     *      * @return All the possible moves the piece can do.     */    public List<Position> posMoves(ChessBoard board, Position pos);    /**     * Gets the value of the piece.     *      * @return The value of the piece.     */    public int value();    /**     * Gets the name of the piece.     *      * @return The name of the piece.     */    public String name();    /**     * Gets the image that this piece represents.     *      * @return The image corresponding to the piece.     */    public Icon image();    /**     * Checks if the piece is white.     *      * @return true if this piece is white, false otherwise.     */    public boolean isWhite();}The Piece interface is then implemented by all the Pieces.The King class is probably the most complicated of all pieces. The posMoves() method makes sure the King does not step right into the line of fire, by checking every possible danger in every possible move:import java.util.LinkedList;import java.util.List;import javax.swing.Icon;import javax.swing.ImageIcon;public class King implements Piece {    /**     * The value of a king.     */    private static final int VALUE = 15;    /**     * The name of the piece.     */    private static final String NAME = King;    /**     * The possible moves of the piece.     *      * In each array in the array, there will be two integers. One represents x,     * and the other y.     */    private static final int[][] posMoves = { { 1, 1 }, { 1, 0 }, { 1, -1 },            { -1, 0 }, { -1, 1 }, { -1, -1 }, { 0, 1 }, { 0, -1 } };    /**     * The colour of the piece.     */    private final boolean isWhite;    /**     * The image representing the piece.     */    private final Icon image;    /**     * Constructs a new King object.     *      * @param isWhite     *            The colour of the piece.     */    public King(boolean isWhite) {        this.isWhite = isWhite;        this.image = new ImageIcon((isWhite ? White  : Black ) + NAME                + .JPG);    }    /**     * {@inheritDoc}     */    @Override    public List<Position> posMoves(ChessBoard board, Position pos) {        List<Position> result = new LinkedList<Position>();        int x = pos.getX();        int y = pos.getY();        for (int[] posMove : posMoves) {            int posX = x + posMove[0];            int posY = y + posMove[1];            if (board.inSize(posX) && board.inSize(posY)) {                Position possibleMove = new Position(x, y);                if (isSafe(board, pos, possibleMove)) {                    result.add(possibleMove);                }            }        }        return result;    }    /**     * Checks if the position at pos is safe for the piece at currentPos.     *      * @param board     *            The board.     * @param currentPos     *            The position where the piece is.     * @param pos     *            The position to check.     * @return true if pos is safe, false otherwise.     */    private boolean isSafe(ChessBoard board, Position currentPos, Position pos) {        board.testMove(currentPos, pos);        for (int i = 1; i <= ChessBoard.SIZE; i++) {            for (int j = 1; j <= ChessBoard.SIZE; j++) {                Position posOfThreat = new Position(i, j);                Piece possibleThreat = board.getPiece(posOfThreat);                if (possibleThreat != null                        && !possibleThreat.posMoves(board, posOfThreat)                                .contains(pos)) {                    return false;                }            }        }        board.stopTest();        return true;    }    /**     * {@inheritDoc}     */    @Override    public int value() {        return VALUE;    }    /**     * {@inheritDoc}     */    @Override    public String name() {        return NAME;    }    /**     * {@inheritDoc}     */    @Override    public Icon image() {        return image;    }    /**     * {@inheritDoc}     */    @Override    public boolean isWhite() {        return isWhite;    }}Then the Rook class, which does the following:Creates a counter and starts moving it in one direction.Each square it lands on:If it is empty, add the square in the results list, and continue.If it is a piece of the same colour, break immediately.If it is a piece of opposite colour, add the square in the results list, and break.Repeat for all the other directions.import java.util.LinkedList;import java.util.List;import javax.swing.Icon;import javax.swing.ImageIcon;public class Rook implements Piece {    /**     * The value of a rook.     */    private static final int VALUE = 5;    /**     * The name of the piece.     */    private static final String NAME = Rook;    /**     * The colour of the piece.     */    private final boolean isWhite;    /**     * The image representing the piece.     */    private final Icon image;    /**     * Constructs a new Rook object.     *      * @param isWhite     *            The colour of the piece.     */    public Rook(boolean isWhite) {        this.isWhite = isWhite;        this.image = new ImageIcon((isWhite ? White  : Black ) + NAME                + .JPG);    }    /**     * {@inheritDoc}     */    @Override    public List<Position> posMoves(ChessBoard board, Position pos) {        List<Position> result = new LinkedList<Position>();        for (int i = 1, max = ChessBoard.SIZE; i < max; i++) {            int x = pos.getX() + i;            int y = pos.getY();            if (x <= ChessBoard.SIZE) {                Position posMove = new Position(x, y);                if (board.getPiece(posMove) == null) {                    result.add(posMove);                } else if (board.getPiece(posMove).isWhite() ^ !isWhite) {                    max = i;                } else {                    result.add(posMove);                    max = i;                }            } else {                max = i;            }        }        for (int i = 1, max = ChessBoard.SIZE; i < max; i++) {            int x = pos.getX();            int y = pos.getY() + i;            if (y <= ChessBoard.SIZE) {                Position posMove = new Position(x, y);                if (board.getPiece(posMove) == null) {                    result.add(posMove);                } else if (board.getPiece(posMove).isWhite() ^ !isWhite) {                    max = i;                } else {                    result.add(posMove);                    max = i;                }            } else {                max = i;            }        }        for (int i = 1, max = ChessBoard.SIZE; i < max; i++) {            int x = pos.getX() - i;            int y = pos.getY();            if (x > 0) {                Position posMove = new Position(x, y);                if (board.getPiece(posMove) == null) {                    result.add(posMove);                } else if (board.getPiece(posMove).isWhite() ^ !isWhite) {                    max = i;                } else {                    result.add(posMove);                    max = i;                }            } else {                max = i;            }        }        for (int i = 1, max = ChessBoard.SIZE; i < max; i++) {            int x = pos.getX();            int y = pos.getY() - i;            if (y > 0) {                Position posMove = new Position(x, y);                if (board.getPiece(posMove) == null) {                    result.add(posMove);                } else if (board.getPiece(posMove).isWhite() ^ !isWhite) {                    max = i;                } else {                    result.add(posMove);                    max = i;                }            } else {                max = i;            }        }        return result;    }    /**     * {@inheritDoc}     */    @Override    public int value() {        return VALUE;    }    /**     * {@inheritDoc}     */    @Override    public String name() {        return NAME;    }    /**     * {@inheritDoc}     */    @Override    public Icon image() {        return image;    }    /**     * {@inheritDoc}     */    @Override    public boolean isWhite() {        return isWhite;    }}Next, the Bishop, which is very similar to the Rook:import java.util.LinkedList;import java.util.List;import javax.swing.Icon;import javax.swing.ImageIcon;public class Bishop implements Piece {    /**     * The value of the bishop.     */    private static final int VALUE = 3;    /**     * The name of the piece.     */    private static final String NAME = Bishop;    /**     * The colour of the piece.     */    private final boolean isWhite;    /**     * The image representing the piece.     */    private final Icon image;    /**     * Constructs a new Bishop object.     *      * @param isWhite     *            The colour of the piece.     */    public Bishop(boolean isWhite) {        this.isWhite = isWhite;        this.image = new ImageIcon((isWhite ? White  : Black ) + NAME                + .JPG);    }    /**     * {@inheritDoc}     */    @Override    public List<Position> posMoves(ChessBoard board, Position pos) {        List<Position> result = new LinkedList<Position>();        for (int i = 1, max = ChessBoard.SIZE; i < max; i++) {            int x = pos.getX() + i;            int y = pos.getY() + i;            if (x <= ChessBoard.SIZE && y <= ChessBoard.SIZE) {                Position posMove = new Position(x, y);                if (board.getPiece(posMove) == null) {                    result.add(posMove);                } else if (board.getPiece(posMove).isWhite() ^ !isWhite) {                    max = i;                } else {                    result.add(posMove);                    max = i;                }            } else {                max = i;            }        }        for (int i = 1, max = ChessBoard.SIZE; i < max; i++) {            int x = pos.getX() + i;            int y = pos.getY() - i;            if (x <= ChessBoard.SIZE && y > 0) {                Position posMove = new Position(x, y);                if (board.getPiece(posMove) == null) {                    result.add(posMove);                } else if (board.getPiece(posMove).isWhite() ^ !isWhite) {                    max = i;                } else {                    result.add(posMove);                    max = i;                }            } else {                max = i;            }        }        for (int i = 1, max = ChessBoard.SIZE; i < max; i++) {            int x = pos.getX() - i;            int y = pos.getY() + i;            if (x > 0 && y <= ChessBoard.SIZE) {                Position posMove = new Position(x, y);                if (board.getPiece(posMove) == null) {                    result.add(posMove);                } else if (board.getPiece(posMove).isWhite() ^ !isWhite) {                    max = i;                } else {                    result.add(posMove);                    max = i;                }            } else {                max = i;            }        }        for (int i = 1, max = ChessBoard.SIZE; i < max; i++) {            int x = pos.getX() - i;            int y = pos.getY() - i;            if (x > 0 && y > 0) {                Position posMove = new Position(x, y);                if (board.getPiece(posMove) == null) {                    result.add(posMove);                } else if (board.getPiece(posMove).isWhite() ^ !isWhite) {                    max = i;                } else {                    result.add(posMove);                    max = i;                }            } else {                max = i;            }        }        return result;    }    /**     * {@inheritDoc}     */    @Override    public int value() {        return VALUE;    }    /**     * {@inheritDoc}     */    @Override    public String name() {        return NAME;    }    /**     * {@inheritDoc}     */    @Override    public Icon image() {        return image;    }    /**     * {@inheritDoc}     */    @Override    public boolean isWhite() {        return isWhite;    }}The Queen pretty much is a combination of a Rook and a Bishop:import java.util.List;import javax.swing.Icon;import javax.swing.ImageIcon;public class Queen implements Piece {    /**     * The value of a queen.     */    private static final int VALUE = 9;    /**     * The name of the piece.     */    private static final String NAME = Queen;    /**     * The colour of the piece.     */    private final boolean isWhite;    /**     * The image representing the piece.     */    private final Icon image;    /**     * Constructs a new Queen object.     *      * @param isWhite     *            The colour of the piece.     */    public Queen(boolean isWhite) {        this.isWhite = isWhite;        this.image = new ImageIcon((isWhite ? White  : Black ) + NAME                + .JPG);    }    /**     * {@inheritDoc}     */    @Override    public List<Position> posMoves(ChessBoard board, Position pos) {        List<Position> result = new Bishop(isWhite).posMoves(board, pos);        result.addAll(new Rook(isWhite).posMoves(board, pos));        return result;    }    /**     * {@inheritDoc}     */    @Override    public int value() {        return VALUE;    }    /**     * {@inheritDoc}     */    @Override    public String name() {        return NAME;    }    /**     * {@inheritDoc}     */    @Override    public Icon image() {        return image;    }    /**     * {@inheritDoc}     */    @Override    public boolean isWhite() {        return isWhite;    }}Next, the Knight, which is probably the easiest, as it can jump over piece:import java.util.LinkedList;import java.util.List;import javax.swing.Icon;import javax.swing.ImageIcon;public class Knight implements Piece {    /**     * The value of a knight.     */    private static final int VALUE = 3;    /**     * The name of the piece.     */    private static final String NAME = Knight;    /**     * The possible moves of the piece.     *      * In each array in the array, there will be two integers. One represents x,     * and the other y.     */    private static final int[][] posMoves = { { 1, 2 }, { -1, 2 }, { 1, -2 },            { -1, -2 }, { 2, 1 }, { 2, -1 }, { -2, 1 }, { -2, -1 } };    /**     * The colour of the piece.     */    private final boolean isWhite;    /**     * The image representing the piece.     */    private final Icon image;    /**     * Constructs a new Knight object.     *      * @param isWhite     *            The colour of the piece.     */    public Knight(boolean isWhite) {        this.isWhite = isWhite;        this.image = new ImageIcon((isWhite ? White  : Black ) + NAME                + .JPG);    }    /**     * {@inheritDoc}     */    @Override    public List<Position> posMoves(ChessBoard board, Position pos) {        List<Position> result = new LinkedList<Position>();        for (int[] posMove : posMoves) {            int posX = pos.getX() + posMove[0];            int posY = pos.getY() + posMove[1];            Position possibleMove = new Position(posX, posY);            if (posX <= ChessBoard.SIZE                    && posY <= ChessBoard.SIZE                    && posX > 0                    && posY > 0                    && (board.getPiece(possibleMove) == null || (board                            .getPiece(possibleMove).isWhite() ^ isWhite))) {                result.add(possibleMove);            }        }        return result;    }    /**     * {@inheritDoc}     */    @Override    public int value() {        return VALUE;    }    /**     * {@inheritDoc}     */    @Override    public String name() {        return NAME;    }    /**     * {@inheritDoc}     */    @Override    public Icon image() {        return image;    }    /**     * {@inheritDoc}     */    @Override    public boolean isWhite() {        return isWhite;    }}Finally, the pawn:import java.util.LinkedList;import java.util.List;import javax.swing.Icon;import javax.swing.ImageIcon;public class Pawn implements Piece {    /**     * The value of a pawn.     */    private static final int VALUE = 1;    /**     * The name of the piece.     */    private static final String NAME = Pawn;    /**     * The colour of the piece.     */    private final boolean isWhite;    /**     * The image representing the piece.     */    private final Icon image;    /**     * Constructs a new Pawn object.     *      * @param isWhite     *            The colour of the piece.     */    public Pawn(boolean isWhite) {        this.isWhite = isWhite;        this.image = new ImageIcon((isWhite ? White  : Black ) + NAME                + .JPG);    }    /**     * {@inheritDoc}     */    @Override    public List<Position> posMoves(ChessBoard board, Position pos) {        // TODO En passant        List<Position> result = new LinkedList<Position>();        // First move possible 2 steps        Position posMove = new Position(pos.getX(), pos.getY()                + (isWhite ? 2 : -2));        Position posMove2 = new Position(pos.getX(), pos.getY()                + (isWhite ? 1 : -1));        if (pos.getY() == (isWhite ? 2 : ChessBoard.SIZE - 2)                && board.getPiece(posMove) == null                && board.getPiece(posMove2) == null) {            result.add(posMove);        }        if (board.getPiece(posMove2) == null) {            result.add(posMove2);        }        // Capture        posMove = new Position(pos.getX() + 1, pos.getY() + (isWhite ? 1 : -1));        posMove2 = new Position(pos.getX() - 1, pos.getY() + (isWhite ? 1 : -1));        if (board.getPiece(posMove) != null                && (board.getPiece(posMove).isWhite() ^ isWhite)) {            result.add(posMove);        }        if (board.getPiece(posMove2) != null                && (board.getPiece(posMove2).isWhite() ^ isWhite)) {            result.add(posMove2);        }        return result;    }    /**     * {@inheritDoc}     */    @Override    public int value() {        return VALUE;    }    /**     * {@inheritDoc}     */    @Override    public String name() {        return NAME;    }    /**     * {@inheritDoc}     */    @Override    public Icon image() {        return image;    }    /**     * {@inheritDoc}     */    @Override    public boolean isWhite() {        return isWhite;    }}Questions:Does it connect well with my ChessBoard class?Does it smell?Am I using JavaDoc correctly and well?Is it as efficient as it could be?
Simple Chess Game, Part 2: The Pieces
java;chess
First observation: you should probably be thinking about separating the model of the game from the presentation of the game.For example:public interface Piece {    public List<Position> posMoves(ChessBoard board, Position pos);    public Icon image();posMoves is about the game in the abstract - that's clearly a representation of the rules of the game (applied to the specific state).  But a javax.swing.Icon ?  That only makes sense in a presentation layer that is specifically using swing icons for something.  So you clearly have an interface here that is representing two very different concerns; which in turn is going to make it hard to create good modules, tests, and so on.(BTW: pos is not a verb.  This should be getMoves(), or calculateMoves(), or....)public class King implements Piece {    private static final int VALUE = 15;    private static final String NAME = King;    private static final int[][] posMoves = { { 1, 1 }, { 1, 0 }, { 1, -1 },        { -1, 0 }, { -1, 1 }, { -1, -1 }, { 0, 1 }, { 0, -1 } };    private final boolean isWhite;    private final Icon image;First thing to call out here is that - more than likely - every piece class is going to share these properties.  That suggests that there should be a common base class that stores this information.That shouldn't be to much of a surprise, if you think about your description for a moment -- The posMoves() method makes sure the King does not step right into the line of fire  What this is really saying is that the king is a piece that has a specialized behavior for testing whether a move to a square is legal (because the king can't move into check).I think you'll want to give some thought to your enumeration of moves.  You are expressing them all as int-pairs, which is reasonable for piece moves, but that idiom doesn't work at all for en passant captures or castling.  I think you'll be better served by actually teasing out a Move interface.  Each piece would have a collection of moves to test.  The int pairs become a RelativePositionMove, but the rules for more complicated situations become EnPassantCaptureMove, and StrongSideCastleMove, and so on.  The piece logic then becomes a lot easier to manage.Did you notice that your implementation of the possible moves for King included all nine directions, without worrying about the position on the board?  There's no reason that you couldn't do that for Rook, Bishop, Queen, and Knight just as easily.class Queen ... {    public List<Position> posMoves(ChessBoard board, Position pos) {        List<Position> result = new Bishop(isWhite).posMoves(board, pos);        result.addAll(new Rook(isWhite).posMoves(board, pos));        return result;    }}It's pretty bad that to figure out how a queen moves, you are creating new pieces every time.  A better implementation here would be to re-use the rook and bishop rules each time...public class Queen implements Piece {    private final Rook fakeRook;    private final Bishop fakeBishop;    public Queen(boolean isWhite) {        this.fakeRook = new Rook(isWhite);        this.fakeBishop = new Bishop(isWhite);        // ...    }    public List<Position> posMoves(ChessBoard board, Position pos) {        List<Position> result = fakeBishop.posMoves(board, pos);        result.addAll(fakeRook.posMoves(board, pos));        return result;    }I think doing it this way helps expose the fact that we aren't really talking about pieces here, but the rules for the pieces; again - it's another hint that the abstractions aren't quite right.But if I ran the zoo, I'd have a canonical set of rook moves, and a canonical set of bishop moves, and the queen would have a set of rules that was the join of these two sets, rather than having the queen pretend to be two different kinds of pieces.(See wikipedia for another hint that the queen should not be treated as a combination bishop and rook)./** * The colour of the piece. */private final boolean isWhite;Umm, true isn't a colour.  Neither is false.  You should be using an enumeration here, rather than a boolean.    this.image = new ImageIcon((isWhite ? White  : Black ) + NAME            + .JPG);This is screaming for an IconFactory, which takes a name and color and returns the correct Icon.