Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Other answers have pointed out almost everything, so I'll just add one thing. It looks to me that you're using exceptions to somehow perform flow control. Exceptions are not control flow mechanismsExceptions are not control flow mechanisms.

public Position(Point from, Point to) {
    if (from.getX() > Constants.BOARD_SIZE || from.getX() < 0
            || from.getY() > Constants.BOARD_SIZE || from.getY() < 0
            || to.getX() > Constants.BOARD_SIZE || to.getX() < 0
            || to.getY() > Constants.BOARD_SIZE || to.getY() < 0) {
        throw new ArrayIndexOutOfBoundsException();
    }

    this.from = from;
    this.to = to;
}

And then

} catch (IndexOutOfBoundsException e) {
    System.out.println("Invalid coordinates - Outside board");
}

I think you should validate that the given coordinates are inside the board before trying to create the Position. This is user input and is perfectly reasonable to do it. You're already validating that ship.getSize() != Utils.distanceBetweenPoints(from, to). You would even be doing that in the Board object itself, instead of then having Position checking for Constants.BOARD_SIZE.

Other answers have pointed out almost everything, so I'll just add one thing. It looks to me that you're using exceptions to somehow perform flow control. Exceptions are not control flow mechanisms.

public Position(Point from, Point to) {
    if (from.getX() > Constants.BOARD_SIZE || from.getX() < 0
            || from.getY() > Constants.BOARD_SIZE || from.getY() < 0
            || to.getX() > Constants.BOARD_SIZE || to.getX() < 0
            || to.getY() > Constants.BOARD_SIZE || to.getY() < 0) {
        throw new ArrayIndexOutOfBoundsException();
    }

    this.from = from;
    this.to = to;
}

And then

} catch (IndexOutOfBoundsException e) {
    System.out.println("Invalid coordinates - Outside board");
}

I think you should validate that the given coordinates are inside the board before trying to create the Position. This is user input and is perfectly reasonable to do it. You're already validating that ship.getSize() != Utils.distanceBetweenPoints(from, to). You would even be doing that in the Board object itself, instead of then having Position checking for Constants.BOARD_SIZE.

Other answers have pointed out almost everything, so I'll just add one thing. It looks to me that you're using exceptions to somehow perform flow control. Exceptions are not control flow mechanisms.

public Position(Point from, Point to) {
    if (from.getX() > Constants.BOARD_SIZE || from.getX() < 0
            || from.getY() > Constants.BOARD_SIZE || from.getY() < 0
            || to.getX() > Constants.BOARD_SIZE || to.getX() < 0
            || to.getY() > Constants.BOARD_SIZE || to.getY() < 0) {
        throw new ArrayIndexOutOfBoundsException();
    }

    this.from = from;
    this.to = to;
}

And then

} catch (IndexOutOfBoundsException e) {
    System.out.println("Invalid coordinates - Outside board");
}

I think you should validate that the given coordinates are inside the board before trying to create the Position. This is user input and is perfectly reasonable to do it. You're already validating that ship.getSize() != Utils.distanceBetweenPoints(from, to). You would even be doing that in the Board object itself, instead of then having Position checking for Constants.BOARD_SIZE.

Source Link

Other answers have pointed out almost everything, so I'll just add one thing. It looks to me that you're using exceptions to somehow perform flow control. Exceptions are not control flow mechanisms.

public Position(Point from, Point to) {
    if (from.getX() > Constants.BOARD_SIZE || from.getX() < 0
            || from.getY() > Constants.BOARD_SIZE || from.getY() < 0
            || to.getX() > Constants.BOARD_SIZE || to.getX() < 0
            || to.getY() > Constants.BOARD_SIZE || to.getY() < 0) {
        throw new ArrayIndexOutOfBoundsException();
    }

    this.from = from;
    this.to = to;
}

And then

} catch (IndexOutOfBoundsException e) {
    System.out.println("Invalid coordinates - Outside board");
}

I think you should validate that the given coordinates are inside the board before trying to create the Position. This is user input and is perfectly reasonable to do it. You're already validating that ship.getSize() != Utils.distanceBetweenPoints(from, to). You would even be doing that in the Board object itself, instead of then having Position checking for Constants.BOARD_SIZE.