Skip to main content
Tweeted twitter.com/StackCodeReview/status/677328027072753664
deleted 3 characters in body; edited title
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

How can I remove all these else-ifs that return Checking if a chess square isChess space has been attacked?

I have this list of at least seven else-if, when. When their condition is met, I change the same boolean boolean is_attacked to truetrue. This should be simple but I can't figure out in what way I can achieve the same onin a more readable way.

if(request_is_attacked){
    if(as_knight){
        if(piece_abs_val==2){//knight
            is_attacked=true;
        }
    }else if(piece_abs_val==6){//king
        if(i==0){
            is_attacked=true;
        }
    }else if(piece_abs_val==5){//queen
        is_attacked=true;
    }else if(piece_direction%2){
        if(piece_abs_val==4){//rook
            is_attacked=true;
        }
    }else if(piece_abs_val==3){//bishop
        is_attacked=true;
    }else if(i==0 && piece_abs_val==1){
        if(piece_val!=-1){//w_pawn
            if(piece_direction==4 || piece_direction==6){
                is_attacked=true;
            }
        }else{//b_pawn
            if(piece_direction==2 || piece_direction==8){
                is_attacked=true;
            }
        }
    }
}

For more context, this is happening inside a loop (hence the i), when the i is the first it means it is one square apart (i==0), so pawnsPawns and kingsKings use this, but this information shouldn't matter much.

Also, when piece_direction%2 is truetrue, we are moving in a + (that's why I check for rooksRooks), else it means we are in moving x direction, so I proceed to check for bishopsBishops and pawnsPawns.

How can I remove all these else-ifs that return if a chess square is attacked?

I have this list of at least seven else-if, when their condition is met, I change the same boolean is_attacked to true. This should be simple but I can't figure out in what way I can achieve the same on a more readable way.

if(request_is_attacked){
    if(as_knight){
        if(piece_abs_val==2){//knight
            is_attacked=true;
        }
    }else if(piece_abs_val==6){//king
        if(i==0){
            is_attacked=true;
        }
    }else if(piece_abs_val==5){//queen
        is_attacked=true;
    }else if(piece_direction%2){
        if(piece_abs_val==4){//rook
            is_attacked=true;
        }
    }else if(piece_abs_val==3){//bishop
        is_attacked=true;
    }else if(i==0 && piece_abs_val==1){
        if(piece_val!=-1){//w_pawn
            if(piece_direction==4 || piece_direction==6){
                is_attacked=true;
            }
        }else{//b_pawn
            if(piece_direction==2 || piece_direction==8){
                is_attacked=true;
            }
        }
    }
}

For more context, this is happening inside a loop (hence the i), when the i is the first it means it is one square apart (i==0), so pawns and kings use this, but this information shouldn't matter much.

Also, when piece_direction%2 is true we are moving in a + (that's why I check for rooks), else it means we are in moving x direction, so I proceed to check for bishops and pawns.

Checking if a Chess space has been attacked

I have this list of at least seven else-if. When their condition is met, I change the same boolean is_attacked to true. This should be simple but I can't figure out in what way I can achieve the same in a more readable way.

if(request_is_attacked){
    if(as_knight){
        if(piece_abs_val==2){//knight
            is_attacked=true;
        }
    }else if(piece_abs_val==6){//king
        if(i==0){
            is_attacked=true;
        }
    }else if(piece_abs_val==5){//queen
        is_attacked=true;
    }else if(piece_direction%2){
        if(piece_abs_val==4){//rook
            is_attacked=true;
        }
    }else if(piece_abs_val==3){//bishop
        is_attacked=true;
    }else if(i==0 && piece_abs_val==1){
        if(piece_val!=-1){//w_pawn
            if(piece_direction==4 || piece_direction==6){
                is_attacked=true;
            }
        }else{//b_pawn
            if(piece_direction==2 || piece_direction==8){
                is_attacked=true;
            }
        }
    }
}

For more context, this is happening inside a loop (hence the i), when the i is the first it means it is one square apart (i==0), so Pawns and Kings use this, but this information shouldn't matter much.

Also, when piece_direction%2 is true, we are moving in a + (that's why I check for Rooks), else it means we are in moving x direction, so I proceed to check for Bishops and Pawns.

edited title
Link
ajax333221
  • 264
  • 1
  • 14

How can I remove all these else-ifs that when true, they all do the samereturn if a chess square is attacked?

added 190 characters in body
Source Link
ajax333221
  • 264
  • 1
  • 14

How can a bunch ofI remove all these else-ifs be refactoredthat when true, they all do the same?

I have this list of at least seven else-if, when their condition is met, I change the same boolean is_attacked to true. This should be simple but I can't figure out in what way I can achieve the same on a more readable way.

if(request_is_attacked){
    if(as_knight){
        if(piece_abs_val==2){//knight
            is_attacked=true;
        }
    }else if(piece_abs_val==6){//king
        if(i==0){
            is_attacked=true;
        }
    }else if(piece_abs_val==5){//queen
        is_attacked=true;
    }else if(piece_direction%2){
        if(piece_abs_val==4){//rook
            is_attacked=true;
        }
    }else if(piece_abs_val==3){//bishop
        is_attacked=true;
    }else if(i==0 && piece_abs_val==1){
        if(piece_val!=-1){//w_pawn
            if(piece_direction==4 || piece_direction==6){
                is_attacked=true;
            }
        }else{//b_pawn
            if(piece_direction==2 || piece_direction==8){
                is_attacked=true;
            }
        }
    }
}

For more context, this is happening inside a loop (hence the i), when the i is the first it means it is one square apart (i==0), so pawns and kings use this, but this information shouldn't matter much.

Also, when piece_direction%2 is true we are moving in a + (that's why I check for rooks), else it means we are in moving x direction, so I proceed to check for bishops and pawns.

How can a bunch of else-ifs be refactored?

I have this list of at least seven else-if, when their condition is met, I change the same boolean is_attacked to true. This should be simple but I can't figure out in what way I can achieve the same on a more readable way.

if(request_is_attacked){
    if(as_knight){
        if(piece_abs_val==2){//knight
            is_attacked=true;
        }
    }else if(piece_abs_val==6){//king
        if(i==0){
            is_attacked=true;
        }
    }else if(piece_abs_val==5){//queen
        is_attacked=true;
    }else if(piece_direction%2){
        if(piece_abs_val==4){//rook
            is_attacked=true;
        }
    }else if(piece_abs_val==3){//bishop
        is_attacked=true;
    }else if(i==0 && piece_abs_val==1){
        if(piece_val!=-1){//w_pawn
            if(piece_direction==4 || piece_direction==6){
                is_attacked=true;
            }
        }else{//b_pawn
            if(piece_direction==2 || piece_direction==8){
                is_attacked=true;
            }
        }
    }
}

For more context, this is happening inside a loop (hence the i), when the i is the first it means it is one square apart (i==0), so pawns and kings use this, but this information shouldn't matter much.

How can I remove all these else-ifs that when true, they all do the same?

I have this list of at least seven else-if, when their condition is met, I change the same boolean is_attacked to true. This should be simple but I can't figure out in what way I can achieve the same on a more readable way.

if(request_is_attacked){
    if(as_knight){
        if(piece_abs_val==2){//knight
            is_attacked=true;
        }
    }else if(piece_abs_val==6){//king
        if(i==0){
            is_attacked=true;
        }
    }else if(piece_abs_val==5){//queen
        is_attacked=true;
    }else if(piece_direction%2){
        if(piece_abs_val==4){//rook
            is_attacked=true;
        }
    }else if(piece_abs_val==3){//bishop
        is_attacked=true;
    }else if(i==0 && piece_abs_val==1){
        if(piece_val!=-1){//w_pawn
            if(piece_direction==4 || piece_direction==6){
                is_attacked=true;
            }
        }else{//b_pawn
            if(piece_direction==2 || piece_direction==8){
                is_attacked=true;
            }
        }
    }
}

For more context, this is happening inside a loop (hence the i), when the i is the first it means it is one square apart (i==0), so pawns and kings use this, but this information shouldn't matter much.

Also, when piece_direction%2 is true we are moving in a + (that's why I check for rooks), else it means we are in moving x direction, so I proceed to check for bishops and pawns.

Source Link
ajax333221
  • 264
  • 1
  • 14
Loading