patterncsharpMinor
Searching all diagonals of a 2D M x M array
Viewed 0 times
arraydiagonalssearchingall
Problem
I've started writing a piece of code to help me search for an object in all the objects found in the diagonals of an M x M 2D array.
Though the code works, I'd like to know if there is a way I can improve on it or there exists a different technique I could use to achieve the same result with fewer lines of code. I just don't want to re-invent the wheel.
Though the code works, I'd like to know if there is a way I can improve on it or there exists a different technique I could use to achieve the same result with fewer lines of code. I just don't want to re-invent the wheel.
// board must be a square matrix
bool allDiagonals(string[] board, string strToFind)
{
string s = "";
for (int col = board[0].Length - 1; col >= 0; col--)
{
int row = 0;
int j = col;
s = "";
while (j 0; row--)
{
int colmin = 0;
s = "";
for (int i = row; i = 0)
{
s += board[row++][j--];
}
if (s.Contains(strToFind))
return true;
}
for (int row = board.Length - 1; row > 0; row--)
{
s = "";
int colmax = board[0].Length - 1;
//scan from bottom right to main diagonal
for (int i = row; i < board.Length; i++)
{
s += board[i][colmax--];
}
if (s.Contains(strToFind))
return true;
}
return false;
}Solution
This reminds me of what I wrote a while ago for my Tic Tac Toe Ultimate game.
In case I get anything wrong, here is a relevant part of my original code:
When making this in C#, you can return
I think it's a bad idea that you're currently using String concatenation first, and then checking if the string contains the char. Instead you can indeed as Carsten points out in the comments use a state machine for that. It only needs to be about an
Anyways, back to the main part, here's what I would try to do:
Whenever I'm using C#, I love using the wonderful
Then in your
Please note that I have not tested this code, and as I haven't done any C# development since last time I might have mixed something up. I also might have mixed up X and Y, but that happens for me in Java too every now and then.
Other Suggestions:
-
Don't use
-
As your variable is named
In case I get anything wrong, here is a relevant part of my original code:
// Scan diagonals for a winner: Bottom-right
for (int yy = 0; yy loopAdd(HasSub board,
int xx, int yy, int dx, int dy) {
List winnables = new ArrayList<>();
Winnable tile;
do {
tile = board.getSub(xx, yy);
xx += dx;
yy += dy;
if (tile != null)
winnables.add(tile);
}
while (tile != null);
return winnables;
}When making this in C#, you can return
IEnumerable. The idea of the method is to start at a specific point and to iterate until you're outside the valid area.I think it's a bad idea that you're currently using String concatenation first, and then checking if the string contains the char. Instead you can indeed as Carsten points out in the comments use a state machine for that. It only needs to be about an
int really that keeps track of how many consecutive matches you've had so far, and when it reaches a non-match it starts over at zero. When it reaches the string length, it returns true.Anyways, back to the main part, here's what I would try to do:
private IEnumerable loopOver(string[] board,
Point position, Point delta) {
char value;
while (true) {
if (!charIsInsideBoard(board, position))
break;
value = getCharInBoard(board, position);
position.x += delta.x;
position.y += delta.y;
yield return value;
}
}Whenever I'm using C#, I love using the wonderful
yield keyword.private boolean contains(IEnumerable loop, string searchFor) {
int matches = 0;
foreach (char ch in loop) {
if (ch == searchFor[matches]) {
matches++;
}
else {
matches = 0;
if (ch == searchFor[matches]) {
matches++;
}
}
if (matches == searchFor.Length) {
return true;
}
}
return false;
}Then in your
allDiagonals method (which really could be renamed to searchAllDiagonals) all you need to do is to call the contains and loopOver methods a couple of times.boolean searchAllDiagonals(string[] board, string searchFor)
{
boolean result = false;
for (int i = 0; i < board[0].Length; i++) {
result = result || contains(loopOver(board, new Point(i, 0), new Point(1, 1), searchFor);
}
for (int i = 0; i < board.Length; i++) {
result = result || contains(loopOver(board, new Point(0, i), new Point(1, 0), searchFor);
}
...
}Please note that I have not tested this code, and as I haven't done any C# development since last time I might have mixed something up. I also might have mixed up X and Y, but that happens for me in Java too every now and then.
Other Suggestions:
-
Don't use
i and j when you're using nested loops. row and col are so much better, or x and y. I myself use xx and yy sometimes for loops when I already have another x / y variable.-
As your variable is named
board, what do I know, perhaps your whole point is to check for a winner in a Tic Tac Toe game. If a game is your context, then I'd recommend not using string[]. Use a BoardField[][] variable instead, or preferably wrap such a 2D array in a Board class (which can contain the charIsInsideBoard and getCharInBoard methods).Code Snippets
// Scan diagonals for a winner: Bottom-right
for (int yy = 0; yy < board.getSizeY(); yy++) {
newWin(conds, consecutive, loopAdd(board, 0, yy, 1, 1));
}
private static List<Winnable> loopAdd(HasSub<? extends Winnable> board,
int xx, int yy, int dx, int dy) {
List<Winnable> winnables = new ArrayList<>();
Winnable tile;
do {
tile = board.getSub(xx, yy);
xx += dx;
yy += dy;
if (tile != null)
winnables.add(tile);
}
while (tile != null);
return winnables;
}private IEnumerable<char> loopOver(string[] board,
Point position, Point delta) {
char value;
while (true) {
if (!charIsInsideBoard(board, position))
break;
value = getCharInBoard(board, position);
position.x += delta.x;
position.y += delta.y;
yield return value;
}
}private boolean contains(IEnumerable<char> loop, string searchFor) {
int matches = 0;
foreach (char ch in loop) {
if (ch == searchFor[matches]) {
matches++;
}
else {
matches = 0;
if (ch == searchFor[matches]) {
matches++;
}
}
if (matches == searchFor.Length) {
return true;
}
}
return false;
}boolean searchAllDiagonals(string[] board, string searchFor)
{
boolean result = false;
for (int i = 0; i < board[0].Length; i++) {
result = result || contains(loopOver(board, new Point(i, 0), new Point(1, 1), searchFor);
}
for (int i = 0; i < board.Length; i++) {
result = result || contains(loopOver(board, new Point(0, i), new Point(1, 0), searchFor);
}
...
}Context
StackExchange Code Review Q#56092, answer score: 5
Revisions (0)
No revisions yet.