HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Connect Four class project

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
homeworkcodereviewconnect-fourjavastackoverflow
projectfourconnectclass

Problem

I am in a computer coding class and we have to make a project for Java.

Is there any way we can better our existing connect four code?

```
import java.util.*;

public class Connect4
{

public static boolean putDisk(char[][] field, int column, char color) {

if (field[0][column] != ' ')
return false;

for (int row = 0; row = 4) {

return field[row][column];
}
}
}

return ' ';
}

private static char getWinnerInColumns(char[][] field) {

for (int column = 0; column = 4) {

return field[row][column];
}
}
}

return ' ';
}

private static char getWinnerInDiagonals(char[][] field) {

for (int column = 0; column = 7) break;
if (field[row][column+row] != ' ' &&
field[row-1][column + row - 1] == field[row][column+row])
++count;
else
count = 1;
if (count >= 4) return field[row][column+row];
}
}

for (int row = 0; row = 7) break;
if (field[row + column][column] != ' ' &&
field[row+column - 1][column - 1] == field[row + column][column])
++count;
else
count = 1;
if (count >= 4) return field[row + column][column];
}
}

for (int column = 0; column = 4) return field[row][column-row];
}
}

for (int row = 0; row = 0; --column) {

if (column - row = 4) return field[column - row][column];
}
}

return ' ';
}

public static char getWinner(char[][] field) {
char winner = getWinnerInRows(field);
if (winner != ' ') return winner;
winner = getWinnerInColumns(field);
if (winner != ' ') return winner;
winner = getWin

Solution

Code Duplication

Your code has lots of parts which are similar. These can be easily refactored to be
the same:

private static char getWinnerInRows(char[][] field) {
    // ..
            if (field[row][column] != ' ' && field[row][column] == field[row][column - 1])
                ++count;
            else
                count = 1;
 // ...
private static char getWinnerInColumns(char[][] field) {
 // ...
            if (field[row][column] != ' ' && field[row][column] == field[row - 1][column])
                ++count;
            else
                count = 1;
 // ...
private static char getWinnerInDiagonals(char[][] field) {
 // ...
            if (column + row >= 7)
                break;
            if (field[row][column + row] != ' ' && field[row - 1][column + row - 1] == field[row][column + row])
                ++count;
            else
                count = 1;
 // some more variants


This code looks almost the same and it is hard do see what the differences are.

We could try to refactor this so that the common logic is in one place and the differences stand out better:
To do so we extract the actual index calculations to new local variables that have the
same names in all methods:

private static char getWinnerInRows(char[][] field) {
 // ...
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row;
            int neighbourColumn = column - 1;
            if (field[currentRow][currentColumn] != ' '
                    && field[currentRow][currentColumn] == field[neighbourRow][neighbourColumn])
                ++count;
            else
                count = 1;
 // ...
private static char getWinnerInColumns(char[][] field) {
 // ...     
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row - 1;
            int neighbourColumn = column;
            if (field[currentRow][currentColumn] != ' ' && field[currentRow][currentColumn] == field[neighbourRow][neighbourColumn])
                ++count;
            else
                count = 1;
 // ...
private static char getWinnerInDiagonals(char[][] field) {
 // ...   
            int currentRow = row;
            int currentColumn = column + row;
            int neighbourRow = row - 1;
            int neighbourColumn = column + row - 1;
            if (field[currentRow][currentColumn] != ' ' && field[neighbourRow][neighbourColumn] == field[currentRow][currentColumn])
                ++count;
            else
                count = 1;


In the method getWinnerInDiagonals() we have to switch the order of operands in the
second compare to match it with the methods getWinnerInRows() and getWinnerInColumns()
(or vice versa):

private static char getWinnerInDiagonals(char[][] field) {
 // ...   
            int currentRow = row;
            int currentColumn = column + row;
            int neighbourRow = row - 1;
            int neighbourColumn = column + row - 1;
            if (field[currentRow][currentColumn] != ' ' && field[currentRow][currentColumn]) == field[neighbourRow][neighbourColumn]
                ++count;
            else
                count = 1;


Do so at all places where this 4 lines exist.

Now select this 4 lines and use your IDEs refactoring extract method.

This will move this lines to a new method and replace all occurences with the call
to the new method:

private static int countNeighbourIsSame(char[][] field, int count, int currentRow, int currentColumn,
        int neighbourRow, int neighbourColumn) {
    if (field[currentRow][currentColumn] != ' '
            && field[neighbourRow][neighbourColumn] == field[currentRow][currentColumn])
        ++count;
    else
        count = 1;
    return count;
}

private static char getWinnerInRows(char[][] field) {
 // ...
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row;
            int neighbourColumn = column - 1;
            count = countNeighbourIsSame(field, count, currentRow, currentColumn, neighbourRow, neighbourColumn);
 // ...
private static char getWinnerInColumns(char[][] field) {
 // ...     
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row - 1;
            int neighbourColumn = column;
            count = countNeighbourIsSame(field, count, currentRow, currentColumn, neighbourRow, neighbourColumn);
 // ...
private static char getWinnerInDiagonals(char[][] field) {
 // ...   
            int currentRow = row;
            int currentColumn = column + row;
            int neighbourRow = row - 1;
            int neighbourColumn = column + row - 1;
            count = countNeighbourIsSame(field, count, currentRow, currentColumn, neighbourRow, neighbourColumn);


Now you still have more lines that you had before, but now these lines contain specific
information unique to the method (or block) they are placed it. The ac

Code Snippets

private static char getWinnerInRows(char[][] field) {
    // ..
            if (field[row][column] != ' ' && field[row][column] == field[row][column - 1])
                ++count;
            else
                count = 1;
 // ...
private static char getWinnerInColumns(char[][] field) {
 // ...
            if (field[row][column] != ' ' && field[row][column] == field[row - 1][column])
                ++count;
            else
                count = 1;
 // ...
private static char getWinnerInDiagonals(char[][] field) {
 // ...
            if (column + row >= 7)
                break;
            if (field[row][column + row] != ' ' && field[row - 1][column + row - 1] == field[row][column + row])
                ++count;
            else
                count = 1;
 // some more variants
private static char getWinnerInRows(char[][] field) {
 // ...
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row;
            int neighbourColumn = column - 1;
            if (field[currentRow][currentColumn] != ' '
                    && field[currentRow][currentColumn] == field[neighbourRow][neighbourColumn])
                ++count;
            else
                count = 1;
 // ...
private static char getWinnerInColumns(char[][] field) {
 // ...     
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row - 1;
            int neighbourColumn = column;
            if (field[currentRow][currentColumn] != ' ' && field[currentRow][currentColumn] == field[neighbourRow][neighbourColumn])
                ++count;
            else
                count = 1;
 // ...
private static char getWinnerInDiagonals(char[][] field) {
 // ...   
            int currentRow = row;
            int currentColumn = column + row;
            int neighbourRow = row - 1;
            int neighbourColumn = column + row - 1;
            if (field[currentRow][currentColumn] != ' ' && field[neighbourRow][neighbourColumn] == field[currentRow][currentColumn])
                ++count;
            else
                count = 1;
private static char getWinnerInDiagonals(char[][] field) {
 // ...   
            int currentRow = row;
            int currentColumn = column + row;
            int neighbourRow = row - 1;
            int neighbourColumn = column + row - 1;
            if (field[currentRow][currentColumn] != ' ' && field[currentRow][currentColumn]) == field[neighbourRow][neighbourColumn]
                ++count;
            else
                count = 1;
private static int countNeighbourIsSame(char[][] field, int count, int currentRow, int currentColumn,
        int neighbourRow, int neighbourColumn) {
    if (field[currentRow][currentColumn] != ' '
            && field[neighbourRow][neighbourColumn] == field[currentRow][currentColumn])
        ++count;
    else
        count = 1;
    return count;
}

private static char getWinnerInRows(char[][] field) {
 // ...
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row;
            int neighbourColumn = column - 1;
            count = countNeighbourIsSame(field, count, currentRow, currentColumn, neighbourRow, neighbourColumn);
 // ...
private static char getWinnerInColumns(char[][] field) {
 // ...     
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row - 1;
            int neighbourColumn = column;
            count = countNeighbourIsSame(field, count, currentRow, currentColumn, neighbourRow, neighbourColumn);
 // ...
private static char getWinnerInDiagonals(char[][] field) {
 // ...   
            int currentRow = row;
            int currentColumn = column + row;
            int neighbourRow = row - 1;
            int neighbourColumn = column + row - 1;
            count = countNeighbourIsSame(field, count, currentRow, currentColumn, neighbourRow, neighbourColumn);
private static char getWinnerInRows(char[][] field) {
    for (int row = 0; row < 7; ++row) {
        int count = 0;
        for (int column = 1; column < 7; ++column) {
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row;
            int neighbourColumn = column - 1;
            count = countNeighbourIsSame(field, count, currentRow, currentColumn, neighbourRow, neighbourColumn);
            if (count >= 4) {
                return field[row][column];
            }
private static char getWinnerInColumns(char[][] field) {
    for (int column = 0; column < 7; ++column) {
        int count = 0;
        for (int row = 1; row < 7; ++row) {
            int currentRow = row;
            int currentColumn = column;
            int neighbourRow = row - 1;
            int neighbourColumn = column;
            count = countNeighbourIsSame(field, count, currentRow, currentColumn, neighbourRow, neighbourColumn);
            if (count >= 4) {
                return field[row][column];
            }

Context

StackExchange Code Review Q#157944, answer score: 3

Revisions (0)

No revisions yet.