Longest sequence of same subsequent number in a square matrix

Given an N-by-N grid with each cell either occupied by an 'X', an 'O', or empty, write a program to find the longest sequence of consecutive 'X's either horizontal, vertically, or diagonally. To test your program, you can create a random grid where each cell contains an 'X' or 'O' with probability 1/3.

I would like a review of my answer. I'm not sure it's efficient enough, or even efficient at all. I chose to use numbers for now since they easier to work with. I hope it's all clear.

class Main {
    public static void main(String[] args){
        int n = 4;
        int[][] grid = new int[n][n];
        int max =  Integer.MIN_VALUE;

        for(int i = 0; i  max? hori: max;
            }

            // vertical  instances 
            for(int j = 0; j  max? vert: max;
            }

            // forward diag instances (fdia)
            for(int j = 0; j  max? fdia: max;
            }

            // backward diag instances (bdia) 
            for(int j = 0; j  max? bdia: max;
            }
        }

        System.out.println("max : "+max);
        System.out.println("num : "+num);
    }
}

Let me start out by saying that your solution certainly is efficient enough. You only loop over the grid once to do the 4 checks. And don't check the same sequence twice. (like going left to right AND right to left).

Since this is codereview.stackexchange let's instead start by reviewing the code :)

First of all, there is a mistake in your solution. Diagonally is not the same as on the main diagonals. The simplest example that you won't find in your solution but what I believe does count as a normal diagonal sequence is the following:


O X O

O O X

O O O

This looks like a sequence of lenght 2 to me.

I dissagree with you when you say:


I chose to use numbers for now since they easier to work with.

We can just as well work with char:

char[][] grid = new char[n][n];
...
if(r == 1) grid[i][j] = 'X';
if(r == 2) grid[i][j] = 'O';
...
if(grid[i][j] == 'X')

Doing this also makes it explicit that you're only checking for the X without having to arbitrarily choose if the 'X' is represented by a 1 or a 2 in your grid.

You have put all the code inside the main method. Although it kinda works for this small problem, it's generally a better idea to split your code up into more manageable blocks.

A few easy ones to separate are a grid generator, and a method to print a grid. By splitting this up into 2 functions we can also provide a fixed grid and still print that grid as well.

Printing is easy. We take the same loops as you already have with the same System.out... calls. But without the random setting of the cell.

public static void printGrid(char[][] grid){
    for(int row = 0; row < grid.length; row++){
        for(int col = 0; col < grid[0].length; col++){
            System.out.print(grid[row][col]+" ");
        }
        System.out.println();
    }
}

Notice how I named the loop variables row and col to make it clear what we're looping over.

Randomly generating a grid isn't much harder. We use the same loops, but now we randomly choose one of 3 characters. I'm using _ to represent "emtpy".

public static char[][] generateRandomGrid(int size){
    char[][] grid = new char[size][size];
    for(int row = 0; row < grid.length; row++){
        for(int col = 0; col < grid[0].length; col++){
            // three nums are produced. there is 1/3 chance for each.
            int r = (int)(Math.random()*3);

            // both 'X' and 'O' have 1/3 chance to be produced. 
            if(r == 0) {
                grid[row][col] = '_';
            }
            if(r == 1) {
                grid[row][col] = 'X';
            }
            if(r == 2) {
                grid[row][col] = 'O';
            }
        }
    }
    return grid;
}

Now for the actual checking of the sequences. This part of your code confused me. You have the outer loop with index i. Then depending on what you check the meaning of this i changes to the row/the column/something you ignore entirely.

Instead let's provide 4 specific methods, that each calculate the maximum sequence in a specific direction for a given grid.

The horizontal one is easy. It's basically what you already did:

public static int findMaxHorizontalSequence(char[][] grid){
    int max = 0;
    for(int row = 0; row  max){
                    max = currentSeq;
                }
            } else {
                currentSeq = 0;
            }
        }
    }
    return max;
}

The vertical check is almost the same, except the for loops are swapped.

public static int findMaxVerticalSequence(char[][] grid){
    int max = 0;
    for(int col = 0; col  max){
                    max = currentSeq;
                }
            } else {
                currentSeq = 0;
            }
        }
    }
    return max;
}

You might have noticed by now that I always use grid.length and grid[0].length. This is so that the functions will all work correctly even if the grid was a rectangular shape instead of a square.

For the diagonals we'll have to do a bit more work. The easiest is probably to split it up in 2 loops. One for each of the following lines of starting locations:

X X X ?        O O O O  
O O O O        X O O O  
O O O O        X O O O  
O O O O        ? O O O

We can ignore the spots with a ?. Those diagonals are only 1 char long, so it will already be counted by the horizontal/vertical checks. No need to repeat this.

The code could look like this:

public static int findMaxDownRightSequence(char[][] grid) {
    int max = 0;
    for (int startRow = 0; startRow  max) {
                    max = currentSeq;
                }
            } else {
                currentSeq = 0;
            }
        }
    }
    for (int startCol = 1; startCol  max) {
                    max = currentSeq;
                }
            } else {
                currentSeq = 0;
            }
        }
    }
    return max;
}

But at this point I really start noticing that I'm copy-pasting a lot. This us

StackExchange Code Review Q#160066, answer score: 2