Java 2d array nested loops

Question

Okay, So I have this method I made that searches a 2d array of user input column length and row length comprised of integers. The method is suppose to find 4 alike numbers in rows, columns, and both major and minor diagonals throughout the entire array. The array can be as large as the user would like. My method works, however it is extremely ugly and feels redundant. I feel like there should be a better way to compact my code that I just am not seeing. Can anyone tell me how I may accomplish this? I am trying to self teach myself so I don't really have any help but my own. Please dont bash my ugly code :) haha. My main issue is the check diagonals part... It seems like it could be shorter.

public static boolean isConsecutiveFour(int[][] values) {
    int count = 0;
    int currentElement = 0;

    //count rows
    for (int x = 0; x < values.length; x++) {
        count = 0;
        currentElement = values[x][0];
        for (int y = 0; y < values[x].length; y++) {
            if (currentElement == values[x][y])
                count++;
            currentElement = values[x][y];
        }
        if (count >= 4)
            return true;
    }

    //count columns
    for (int x = 0; x < values[0].length; x++) {
        count = 0;
        currentElement = values[0][x];
        for (int y = 0; y < values.length; y++) {
            if (currentElement == values[y][x])
                count++;
            currentElement = values[y][x];
        }
        if (count >= 4)
            return true;
    }

    //count diagonals
    for (int x = 0; x < values.length; x++) {
        int increaseRow = x;
        for (int y = 0; y < values[x].length; y++) {
            int currentMajElement = values[x][y], currentSubElement = values[x][y];
            int majCount = 0, subCount = 0;
            int increaseColumn = y, decreaseColumn = y;

            //Check major diagonals
            while (increaseRow < values.length && increaseColumn < values[x].length) {
                if (currentMajElement == values[increaseRow][increaseColumn])
                    majCount++;
                currentMajElement = values[increaseRow][increaseColumn];
                increaseColumn++;
                increaseRow++;
            }
                increaseRow = x;
            //Check minor diagonals
            while (increaseRow < values.length && decreaseColumn >= 0)  {
                if (currentSubElement == values[increaseRow][decreaseColumn])
                    subCount++;
                currentSubElement = values[increaseRow][decreaseColumn];
                increaseRow++;
                decreaseColumn--;
            }
            if (majCount >= 4 || subCount >= 4)
                return true;
        }
    }
return false;
}

Answer 1

I think the nicest thing to do is to create a function which separates the counting of the elements of an array from the array itself. For example, right now you have a "counting procedure", which basically looks like this:

public static boolean count(int[] values) {

   int count = 0;
   int currentElement = values[0];
   for(int i = 0; i < values.length; i++) {
     if (currentElement == values[i])
       count++;
     currentElement = values[i];
   }
   return (count >= 4);
}

Once you've separated this out, counting the rows is as simple as

int rows = values.length
int cols = values[0].length
for (int i = 0;i < rows; i++)
  if (count(values[i]))
    return true;

Counting the columns like this is a little trickier. I think the cleanest way to do this is to actually allocate a temporary array and fill it with the correct values. Note that this isn't faster than what you've recommended -- it's only prettier.

int[] tmpArray = new int[rows];
for (int j = 0; j < cols; i++) {
  for (int i = 0; i < rows; i++)
    tmpArray[i] = values[i][j];
  if (count(tmpArray))
    return true;
}

It's not actually clear to me at all if your diagonal counting code works, but now we have a nice tool to write it intuitively. What's tricky is I cannot see the four distinct diagonals you're counting, and what parts of the code handle each one. With the following approach, we do each one individually:

int diagonal_length = min(rows, cols)
tmpArray = new int[diagonal_length];

//first major diagonal
for(int i = 0; i < diagonal_length;i++)
  tmpArray[i] = values[i][i];
if (count(tmpArray))
  return true;

//second major diagonal
for(int i = 0; i < diagonal_length;i++)
  tmpArray[i] = values[i][cols - 1 - i];
if (count(tmpArray))
  return true;

//first minor diagonal
for(int i = 0; i < diagonal_length;i++)
  tmpArray[i] = values[rows - 1 - i][i];
if (count(tmpArray))
  return true;

//second minor diagonal
for(int i = 0; i < diagonal_length;i++)
  tmpArray[i] = values[rows - 1 - i][cols - 1 - i];
if (count(tmpArray))
  return true;

return false;

The big advantage of this proposed rewrite is not in speed or algorithms, simply a matter of code clarity. It's easier to test and examine this code; e.g. the count function can be tested separately. You can also easily print out the tmpArray variable to check that the right thing is being counted at the beginning of count(), and this is a must if you want to be sure this code is working properly.

Important note:: I haven't tested any of this code yet!! Errors are fairly likely, but the idea should work either way.