Question

i am writing chess in java, this code for checking weather there are obstacles for bishop sometimes works and in some cases doesn't. can you please explain me my mistakes thanks!

public boolean checkifEmpty(int fromRow, int fromColumn, int toRow,
            int toColumn, Figure[][] ChessBoard) {
        int differenceInRows = Math.abs(fromRow - toRow);
        if (differenceInRows == 1 ) {
            return true;
        }

        for (int j = 1; j < differenceInRows; j++) {
            if ((toRow < fromRow) && (toColumn > fromColumn)
                    && ChessBoard[fromRow - j][fromColumn + j] == null) {
                return true;
            } else if ((toRow > fromRow) && (toColumn > fromColumn)
                    && ChessBoard[fromRow + j][fromColumn + j] == null) {
                return true;
            } else if ((toRow > fromRow) && (toColumn < fromColumn)
                    && ChessBoard[fromRow + j][fromColumn - j] == null) {
                return true;
            } else if ((toRow < fromRow) && (toColumn < fromColumn)
                    && ChessBoard[fromRow - j][fromColumn - j] == null) {
                return true;
            }

        }
        return false;
    }
Was it helpful?

Solution 2

I am building up on @AasmundEldhuset answer: You should check if != null and return false immediately. After the loop, just return true as all went well. You could also do away with the first if statement checking if differenceInRows is 1 as you would not enter the loop anymore for a difference of one.

Corrected code:

public boolean checkifEmpty(int fromRow, int fromColumn, int toRow,
        int toColumn, Figure[][] ChessBoard) {
    int differenceInRows = Math.abs(fromRow - toRow);

    for (int j = 1; j < differenceInRows; j++) {
        if ((toRow < fromRow) && (toColumn > fromColumn)
                && ChessBoard[fromRow - j][fromColumn + j] != null) {
            return false;
        } else if ((toRow > fromRow) && (toColumn > fromColumn)
                && ChessBoard[fromRow + j][fromColumn + j] != null) {
            return false;
        } else if ((toRow > fromRow) && (toColumn < fromColumn)
                && ChessBoard[fromRow + j][fromColumn - j] != null) {
            return false;
        } else if ((toRow < fromRow) && (toColumn < fromColumn)
                && ChessBoard[fromRow - j][fromColumn - j] != null) {
            return false;
        }

    }
    return true;
}

A final note to teach you how to fish instead of giving you fish: You should use the debugger within your IDE and step through the code until you find the bug. You should ideally create unit tests using JUnit or equivallent.

Happy programming!

OTHER TIPS

You're very close to getting it right, but you've got one part of your logic the wrong way around: You are returning true immediately when you find an empty cell, which is not correct because there could still be a nonempty cell further out. What conclusion can you draw if you see an empty cell, and what conclusion can you draw if you see a nonempty cell? In which of these cases can you be sure about what the answer should be?

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top