Find number of plus in a 2d array

Problem

CharGrid


The CharGrid class encapsulates a 2-d char array with a couple operations.

int countPlus()


Look for a '+' pattern in the grid made with repetitions of a character. A '+' is made of single character in the middle and four "arms" extending out up, down, left, and right. The arms start with the middle char and extend until the first different character or grid edge. To count as a '+', all the arms should have two or more chars and should all be the same length. For example, the grid below contains exactly 2 +'s: the p-plus in the top left and the x-plus in the top right. The y in the bottom right doesn't work because the top, right, and left arms would be length 2, but the bottom arm would be length 3.

p      
  p    x 
ppppp xxx
  p  y x 
  p yyy  
zzzzzyzzz
  xx y

Plusses may be formed using any character such as digits or punctuation marks, they do not have to be letters. Just treat all chars the same and your code should work fine.

I know as a matter of fact that there are many areas upon which my code could be improved. However, rather than asking other people to correct my code, I would much prefer to be given hints or general principles as to how my code could be improved. I think this way, me, and everyone that read this post could get more out of this exercise this way.

So could someone please provide me with some comments or suggestions? Critics of any level and kind are welcomed! Feel free to pick apart my code!

CharGrid Class:

```
import java.util.ArrayList;
import java.util.List;

public class CharGrid {

private char[][] grid;

/**
* Constructs a new CharGrid with the given grid. Does not make a copy.
*
* @param grid
*/
public CharGrid(char[][] grid) {
this.grid = grid;
}

int countPlus() {

//do not accept grid that has length or width less than 3
if (grid.length 0;
}

/**
* to find out the radius length of the pl

I would much prefer to be given hints or general principles as to how my code could be improved. I think this way, me, and everyone that read this post could get more out of this exercise this way.

I don't know that I agree with this, but that's more of a meta discussion. This is me trying to follow that. I may miss in both directions though (not providing enough discussion to make my points or providing too much for what you want).

//do not accept grid that has length or width less than 3

Why not?

if (grid.length < 3 || grid[0].length < 3) {
            return 0;
        }

You might consider that zero is a perfectly good answer to the problem. If a dimension is less than three, how many pluses could you possibly find?

numberOfPlus += isPlus(xCoordinate, yCoordinate, grid[yCoordinate][xCoordinate]) ? 1 : 0;

Why do numberOfPlus += 0? It's effectively a no-op. So why is this better than

if (isPlus(xCoordinate, yCoordinate, grid[yCoordinate][xCoordinate])) {
                    numberOfPlus++;
                }

The latter version does the same number of comparisons and fewer additions. I would also find it easier to read.

int currentRadius = radiusOfPlus;

You don't actually need to make a new variable for this. The radiusOfPlus variable can be modified and you never need both values at once.

boolean topInBound = isInBounds(top);
        boolean topSameCharacter = false;
        if (topInBound) {
            topSameCharacter = isSameCharacter(top, middleCharacter);
        }

You don't use topInBound outside this block. And only use it once after setting it. There is an obvious way to drop a line from it. And you could actually drop the whole thing down to one if you define the helper method properly. Note that you always use isInBounds and isSameCharacter together such that topSameCharacter or whatever is only true if both are.

You do this same pattern four times. Plus another eight times with a similar pattern later.

if (Boolean.TRUE.equals(topSameCharacter) && Boolean.TRUE.equals(bottomSameCharacter)
                && Boolean.TRUE.equals(leftSameCharacter) && Boolean.TRUE.equals(rightSameCharacter)) {

The equal method returns a Boolean. So really, Boolean.TRUE.equals(topSameCharacter) is just a complicated way of writing topSameCharacter.

//else if all four surroudning space contain different character

I find this location for the comment confusing. I'd put it after the next couple lines instead so that it is inside the block to which it applies. Also, note the spelling of "surrounding".

} else if (Boolean.FALSE.equals(topSameCharacter) && Boolean.FALSE.equals(bottomSameCharacter)
                && Boolean.FALSE.equals(leftSameCharacter) && Boolean.FALSE.equals(rightSameCharacter)) {

Similarly, Boolean.FALSE.equals(topSameCharacter) is the same as !topSameCharacter.

What happens if you add && radiusOfPlus > 1 to that expression and remove all the code inside the {}? What will it return if that makes it false? What if it stays true?

StackExchange Code Review Q#131140, answer score: 3