I have this method to check if a move is possible in Reversi. The method works as it should, but it doesn't feel right for some reason. It feels very repetitive. I have tried to make it less repetitive by splitting it up in other methods, but that didn't make it any better.
public bool MovePossible(int y, int x)
{
//check if position is in field
if (y > 7 || x > 7 || y < 0 || x < 0)
{
return false;
}
//check if spot is not occupied
if(Bord[y, x] != Color.None)
{
return false;
}
//get color of current turn
Color color = InTurn;
//set oppesite color
Color oppositeColor = Color.Black;
if (color == Color.Black) oppositeColor = Color.White;
//check: X, Y + 1
if (y + 1 <= 7)
{
if(Bord[y + 1, x] == oppositeColor)
{
for (int i = y + 2; i <= 7; i++)
{
if(Bord[i, x] == color)
{
return true;
}
}
}
}
//check: X, Y - 1
if (y - 1 >= 0)
{
if (Bord[y - 1, x] == oppositeColor)
{
for (int i = y - 2; i >= 0; i--)
{
if (Bord[i, x] == color)
{
return true;
}
}
}
}
//check: X + 1, Y
if (x + 1 <= 7)
{
if (Bord[y, x + 1] == oppositeColor)
{
for (int i = x + 2; i <= 7; i++)
{
if (Bord[y, i] == color)
{
return true;
}
}
}
}
//check: X - 1, Y
if (x - 1 >= 0)
{
if (Bord[y, x - 1] == oppositeColor)
{
for (int i = x - 2; i >= 0; i--)
{
if (Bord[y, i] == color)
{
return true;
}
}
}
}
//check: X + 1, Y + 1
if (y + 1 <= 7 && x + 1 <= 7)
{
if (Bord[y + 1, x + 1] == oppositeColor)
{
var i = y + 2;
var j = x + 2;
while (i <= 7 && j <= 7)
{
if (Bord[i, j] == color)
{
return true;
}
i++;
j++;
}
}
}
//check: X - 1, Y - 1
if (y - 1 >= 0 && x - 1 >= 0)
{
if (Bord[y - 1, x - 1] == oppositeColor)
{
var i = y - 2;
var j = x - 2;
while (i >= 0 && j >= 0)
{
if (Bord[i, j] == color)
{
return true;
}
i--;
j--;
}
}
}
//check: X - 1, Y + 1
if (y + 1 <= 7 && x - 1 >= 0)
{
if (Bord[y + 1, x - 1] == oppositeColor)
{
var i = y + 2;
var j = x - 2;
while(i <= 7 && j >= 0)
{
if (Bord[i, j] == color)
{
return true;
}
i++;
j--;
}
}
}
//check: X + 1, Y - 1
if (x + 1 <= 7 && y - 1 >= 0)
{
if (Bord[y - 1, x + 1] == oppositeColor)
{
var i = y - 2;
var j = x + 2;
while (i >= 0 && j <= 7)
{
if (Bord[i, j] == color)
{
return true;
}
i--;
j++;
}
}
}
return false;
}
I would like to get some feedback on my code and how I can improve on it being less repetitive.
EDIT:
What is the code supposed to do?
The method has to check if a disk can be placed on a grid. It is part of the game Othello (or Reversi). The method has to return true when:
- Y and x are within the field (so between 0 and 7)
- The spot cannot be occupied
- Y and x has a neighbor with the opposite color. For example: I want to place a black disk on
3, 3, either one or more of the following coordinates has to be white:2, 2,2, 3,2, 4,3, 2,3, 4,4, 2,4, 3,4, 4. - The disk from the opposite color has to be surrounded.
An example:
0 1 2 3 4 5 6 7
0 0 0 0 0 0 0 0 0
1 0 0 0 0 0 0 0 0
2 0 0 0 0 0 0 0 0
3 0 0 0 2 1 0 0 0
4 0 0 0 1 2 0 0 0
5 0 0 0 0 0 0 0 0
6 0 0 0 0 0 0 0 0
7 0 0 0 0 0 0 0 0
I want to place a white(1) disk at Y: 2, X: 3. That's valid because the spot is empty, it's next to a black(2) disk and the black disk is now surrounded. When the disk is placed the grid looks like this:
0 1 2 3 4 5 6 7
0 0 0 0 0 0 0 0 0
1 0 0 0 0 0 0 0 0
2 0 0 0 1 0 0 0 0
3 0 0 0 2 1 0 0 0
4 0 0 0 1 2 0 0 0
5 0 0 0 0 0 0 0 0
6 0 0 0 0 0 0 0 0
7 0 0 0 0 0 0 0 0
Because the black disk is now surrounded by the white disks the disk becomes a white disk (the one at 3, 3), but changing the disk happens in another function and is not important for the question.
Another example:
0 1 2 3 4 5 6 7
0 0 0 0 0 0 0 0 0
1 0 0 0 0 0 0 0 0
2 0 0 0 0 0 0 0 0
3 0 0 0 2 2 2 0 0
4 0 0 0 1 1 0 0 0
5 0 0 0 0 1 0 0 0
6 0 0 0 0 0 0 0 0
7 0 0 0 0 0 0 0 0
Placing a white(1) disk at Y: 4, X: 5 is not valid. It is next to a black(2) disk, but the black disk won't be surrounded, so it's not valid. Y: 2, X: 2 would be a valid move because the black disk at Y: 3, X: 3 is surrounded by Y: 2, X: 2 and Y 4 and X: 4.
