Bad practice? Leaving out for loop condition

Question

I am working on a chess game and right now I am making a method to check from the king, in a direction and returns true if a threat is found, false if no threat is found. This method takes in two ints to increase x and y by. And this method will from the starting location, in the direction given ( (1,0) for up, (1,1) for up&right, (0,1) for down).

My problem is, this method checks the path with a for loop, which will eventually hit the border of the chess game, and find a barrier, or it will find a piece. Either way, that is how the for loop should end. But then what condition would go in the for loop? I feel like it might be bad practice to just leave it out, but then the only logical condition would be to make sure x and y are both in the range of 1-8. But that would leave the code looking like : for(int i = begx + xdiff, j = begy + ydiff; (i < 8 && i > 1) && (j < 8 && j > 1); i += xdiff, j += ydiff) { .. }

This would be the only other option I see, and this in itself seems like bad practice to me also. Because, it looks over complicated and crammed together. And this condition should never be broken, so why would I have it in there if it would never be the reason the loop broke? I don't want other programmers to read my code and think this might be something they have to look out for, when really, I just didn't need a condition there and put it in for double checking.

This is the whole method, for reference:

public boolean incheckPath(Location l1, int xdiff, int ydiff) {
    int begx = l1.getX();
    int begy = l1.getY();

    String team = board[begx][begy].getTeam();

    for(int i = begx + xdiff, j = begy + ydiff; ; i += xdiff, j += ydiff) {
        if(board[i][j].getType() != ' ') {

            if(board[i][j].getType() == '#') {
                return false;
            }
            if(board[i][j].getTeam().equals(team)) {
                return false;
            }
            if(board[i][j].getType() == 'Q' || board[i][j].getType() == 'R') {
                return true;
            }

        }
    }

}

EDIT It has been improved to implement a while loop, but now how could this be improved any? Someone suggested using break and continue so I implemented that where I saw it could be. But, someone said I shouldn't return in a loop. Without making a variable to hold the return value, how would this be possible?

public boolean incheckPath(Location l1, int xdiff, int ydiff) {
    int x = l1.getX();
    int y = l1.getY();

    String team = board[x][y].getTeam();

    while(true) {
        x += xdiff;
        y += ydiff;

        if(board[x][y].getType() == '#' || board[x][y].getTeam().equals(team)) {
            break;
        }

        if(board[x][y].getType() == 'Q' || board[x][y].getType() == 'R') {
            return true;
        }

    }

    return false;

}

Answer 1

What you want to do is more readable as a while loop:

    int i = begx + xdiff;
    int j = begy + ydiff;
    while (true) {
        if(board[i][j].getType() != ' ') {
            if(board[i][j].getType() == '#') {
                return false;
            }
            if(board[i][j].getTeam().equals(team)) {
                return false;
            }
            if(board[i][j].getType() == 'Q' || board[i][j].getType() == 'R') {
                return true;
            }
        }
        i += xdiff; 
        j += ydiff;
    }

Answer 2

If you are determining on your own, based on other code you wrote, when the for loop should end, then you should declare a boolean

boolean keepLooping = true

and set it to false when you want the loop to stop. Note that you have to do this inside the for loop.

Now make that the condition of the loop the variable you just created. In other words it would say ...; keepLooping ; ...

Also, I'm not sure about your implementation of getType, which might return a character. If it returns a string, you probably don't mean to use == to compare strings. You want to use oneString.equals(anotherString) to check if they are equivalent. Using == actually checks to see if they are literally the same variable in memory. Probably this is not what you want.

So in that case your code would look like

if(board[i][j].getType().equals("Q")   ......

But if getType returns a character, this paragraph does not apply.

Answer 3

For this kind of problem, it's best to use a while loop

use a boolean loop = True;

while(loop){...};

You could then use your return statements to quit the loop

Answer 4

I would compare the possible moves from enemy pieces to the King's position, rather than every pieces' possible moves from the King's position. Little picture to demonstrate because I am bad with figuring out how to word that.

       C        C
       C     C   
   "P" C  C       //P is Pawn
 C  C "K" C  C  C //K is King
    C  C  C       //C is where you (potentially) had to check
 C     C     C   



   "P"            //P is Pawn
 C    "K"         //K is King
                  //C is where you (potentially) had to check

Now, I put that situation in my favor, since there is only one piece and it scales with number of pieces. But the reason I would do this is:

once you have the list of possible moves you can use it for other things as well, such as making sure a move is valid, and scoring which move is best. (You may even have a list or array already for this purpose)

I realize that would mean recoding a large chunk of what you have. (Which sounds like it is already working), but thought I would throw it out there.