Skip to main content
Fixed flawed win detection algorithm
Source Link
200_success
  • 145.7k
  • 22
  • 191
  • 481

When checking for a winner, you're examining the entire board. You only need to look at the neighbours ofcoordinates that are collinear with the last chip played, in 7 directions.

import java.util.Arrays;
import java.util.Scanner;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public class ConnectFour {
    private static final char[] players = new char[] { 'X', 'O' };

    private final int width, height;
    private final char[][] grid;
    private int lastCol = -1, lastTop = -1;

    public ConnectFour(int width, int height) {
        this.width = width;
        this.height = height;
        this.grid = new char[height][];
        for (int h = 0; h < height; h++) {
            Arrays.fill(this.grid[h] = new char[width], '.');
        }
    }

    public String toString() {
        return IntStream.range(0, this.width)
                        .mapToObj(Integer::toString)
                        .collect(Collectors.joining()) + "\n" +
               Arrays.stream(this.grid)
                     .map(String::new)
                     .collect(Collectors.joining("\n"));
    }

    /**
     * Prompts the user for a column, repeating until a valid
     * choice is made.
     *
     * @return the chosen column
     */
    public intvoid chooseAndDrop(char symbol, Scanner input) {
        do {
            System.out.print("\nPlayer " + symbol + " turn: ");
            int col = input.nextInt();

            if (! (0 <= col && col < this.width)) {
                System.out.println("Column must be between 0 and " +
                                   (this.width - 1));
                continue;
            }
            for (int h = this.height - 1; h >= 0; h--) {
                if (this.grid[h][col] == '.') {
                    this.grid[h][col]grid[this.lastTop=h][this.lastCol=col] = symbol;
                    return col;return;
                }
            }

            System.out.println("Column " + col + " is full.");
        } while (true);
    }

    private/**
 static final int[][] DIRECTIONS =* newDetects int[][]whether {the last chip played was a winning move.
     */
   new int[]public boolean isWinningPlay() { 
 -1,       if (this.lastCol == -1) },{
            throw new int[]IllegalStateException("No {move -1,has been 0made },yet");
        new}
 int[] { -1, +1 },   char sym = this.grid[this.lastTop][this.lastCol];
        newString int[]streak {= String.format("%c%c%c%c", 0sym, +1sym, }sym, sym);
        newreturn int[]contains(this.horizontal(), {streak) +1,||
 +1 }             contains(this.vertical(), streak) ||
        new int[] { +1,  0 } contains(this.slashDiagonal(), streak) ||
        new int[] { +1, -1 }  contains(this.backslashDiagonal(), streak);
    };

    /**
     * Detects whether the last chipThe playedcontents inof the specified
     *row columncontaining wasthe alast winningplayed movechip.
     */
    publicprivate booleanString isWinningPlayhorizontal(int col) {
        forreturn new String(intthis.grid[this.lastTop]);
 top = 0; top}

 < this.height; top++) {/**
     * The contents of the column containing ifthe (this.grid[top][col]last !=played 'chip.'
     */
    private String vertical() {
        StringBuilder sb = new StringBuilder(this.height);
        for (int dh = 0; dh < DIRECTIONSthis.length;height; d++h++) {
            sb.append(this.grid[h][this.lastCol]);
        int}
 dw = DIRECTIONS[d][0],
     return sb.toString();
    }

    /**
     * The contents of the dh"/" =diagonal DIRECTIONS[d][1];
containing the last played chip
     * (coordinates have a constant sum).
     */
  if (this.isWinningPlay(col, top,private dw,String dh)slashDiagonal() {
        StringBuilder sb = new StringBuilder(this.height);
        for (int h = 0; h < this.height; returnh++) true;{
             int w = this.lastCol + this.lastTop - }h;
            if (0 <= w }
&& w < this.width) {
            return false;   sb.append(this.grid[h][w]);
            }
        }
        assert false;
        return false;sb.toString();
    }

    private/**
 boolean isWinningPlay(int col, int top,* intThe dw,contents intof dh)the {
"\" diagonal containing the last played chip
  char symbol = this.grid[top][col];
* (coordinates have a constant difference).
   for (int consec*/
 = 1; consec <private 4;String consec++backslashDiagonal() {
            intStringBuilder wsb = col + consec *new dw,StringBuilder(this.height);
              for (int h = top + consec * dh;
0; h < this.height; h++) {
       if ( !(0 <= w &&int w <= this.width) ||
lastCol - this.lastTop + h;
            if !(0 <= hw && hw < this.heightwidth) ||{
                 sb.append(this.grid[h][w] != symbol ) {
                return false;;
            }
        }
        return true;sb.toString();
    }

    private static boolean contains(String haystack, String needle) {
        return haystack.indexOf(needle) >= 0;
    }

    public static void main(String[] args) {
        try (Scanner input = new Scanner(System.in)) {
            int height = 6, width = 8;8, moves = height * width;
            ConnectFour board = new ConnectFour(width, height);
            System.out.println("Use 0-" + (width - 1) + " to choose a column.");
            System.out.println(board);

            for (int player = 0; ;moves-- > 0; player = 1 - player) {
                char symbol = players[player];
                int chosenColumn = board.chooseAndDrop(symbol, input);
                System.out.println(board);
                if (board.isWinningPlay(chosenColumn)) {
                    System.out.println("Player " + symbol + " wins!");
                    break;return;
                }
            }
            System.out.println("Game over, no winner.");
        }
    }
}

When checking for a winner, you're examining the entire board. You only need to look at the neighbours of the last chip played, in 7 directions.

import java.util.Arrays;
import java.util.Scanner;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public class ConnectFour {
    private static final char[] players = new char[] { 'X', 'O' };

    private final int width, height;
    private final char[][] grid;

    public ConnectFour(int width, int height) {
        this.width = width;
        this.height = height;
        this.grid = new char[height][];
        for (int h = 0; h < height; h++) {
            Arrays.fill(this.grid[h] = new char[width], '.');
        }
    }

    public String toString() {
        return IntStream.range(0, this.width)
                        .mapToObj(Integer::toString)
                        .collect(Collectors.joining()) + "\n" +
               Arrays.stream(this.grid)
                     .map(String::new)
                     .collect(Collectors.joining("\n"));
    }

    /**
     * Prompts the user for a column, repeating until a valid
     * choice is made.
     *
     * @return the chosen column
     */
    public int chooseAndDrop(char symbol, Scanner input) {
        do {
            System.out.print("\nPlayer " + symbol + " turn: ");
            int col = input.nextInt();

            if (! (0 <= col && col < this.width)) {
                System.out.println("Column must be between 0 and " +
                                   (this.width - 1));
                continue;
            }
            for (int h = this.height - 1; h >= 0; h--) {
                if (this.grid[h][col] == '.') {
                    this.grid[h][col] = symbol;
                    return col;
                }
            }

            System.out.println("Column " + col + " is full.");
        } while (true);
    }

    private static final int[][] DIRECTIONS = new int[][] {
        new int[] { -1, -1 },
        new int[] { -1,  0 },
        new int[] { -1, +1 },
        new int[] {  0, +1 },
        new int[] { +1, +1 },
        new int[] { +1,  0 },
        new int[] { +1, -1 }
    };

    /**
     * Detects whether the last chip played in the specified
     * column was a winning move.
     */
    public boolean isWinningPlay(int col) {
        for (int top = 0; top < this.height; top++) {
            if (this.grid[top][col] != '.') {
                for (int d = 0; d < DIRECTIONS.length; d++) {
                    int dw = DIRECTIONS[d][0],
                        dh = DIRECTIONS[d][1];
                    if (this.isWinningPlay(col, top, dw, dh)) {
                        return true;
                    }
                }
                return false;
            }
        }
        assert false;
        return false;
    }

    private boolean isWinningPlay(int col, int top, int dw, int dh) {
        char symbol = this.grid[top][col];
        for (int consec = 1; consec < 4; consec++) {
            int w = col + consec * dw,
                h = top + consec * dh;
            if ( !(0 <= w && w < this.width) ||
                 !(0 <= h && h < this.height) ||
                 this.grid[h][w] != symbol ) {
                return false;
            }
        }
        return true;
    }

    public static void main(String[] args) {
        try (Scanner input = new Scanner(System.in)) {
            int height = 6, width = 8;
            ConnectFour board = new ConnectFour(width, height);
            System.out.println("Use 0-" + (width - 1) + " to choose a column.");
            System.out.println(board);

            for (int player = 0; ; player = 1 - player) {
                char symbol = players[player];
                int chosenColumn = board.chooseAndDrop(symbol, input);
                System.out.println(board);
                if (board.isWinningPlay(chosenColumn)) {
                    System.out.println("Player " + symbol + " wins!");
                    break;
                }
            }
        }
    }
}

When checking for a winner, you're examining the entire board. You only need to look at the coordinates that are collinear with the last chip played.

import java.util.Arrays;
import java.util.Scanner;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public class ConnectFour {
    private static final char[] players = new char[] { 'X', 'O' };

    private final int width, height;
    private final char[][] grid;
    private int lastCol = -1, lastTop = -1;

    public ConnectFour(int width, int height) {
        this.width = width;
        this.height = height;
        this.grid = new char[height][];
        for (int h = 0; h < height; h++) {
            Arrays.fill(this.grid[h] = new char[width], '.');
        }
    }

    public String toString() {
        return IntStream.range(0, this.width)
                        .mapToObj(Integer::toString)
                        .collect(Collectors.joining()) + "\n" +
               Arrays.stream(this.grid)
                     .map(String::new)
                     .collect(Collectors.joining("\n"));
    }

    /**
     * Prompts the user for a column, repeating until a valid
     * choice is made.
     */
    public void chooseAndDrop(char symbol, Scanner input) {
        do {
            System.out.print("\nPlayer " + symbol + " turn: ");
            int col = input.nextInt();

            if (! (0 <= col && col < this.width)) {
                System.out.println("Column must be between 0 and " +
                                   (this.width - 1));
                continue;
            }
            for (int h = this.height - 1; h >= 0; h--) {
                if (this.grid[h][col] == '.') {
                    this.grid[this.lastTop=h][this.lastCol=col] = symbol;
                    return;
                }
            }

            System.out.println("Column " + col + " is full.");
        } while (true);
    }

    /**
     * Detects whether the last chip played was a winning move.
     */
    public boolean isWinningPlay() { 
        if (this.lastCol == -1) {
            throw new IllegalStateException("No move has been made yet");
        }
        char sym = this.grid[this.lastTop][this.lastCol];
        String streak = String.format("%c%c%c%c", sym, sym, sym, sym);
        return contains(this.horizontal(), streak) ||
               contains(this.vertical(), streak) ||
               contains(this.slashDiagonal(), streak) ||
               contains(this.backslashDiagonal(), streak);
    }

    /**
     * The contents of the row containing the last played chip.
     */
    private String horizontal() {
        return new String(this.grid[this.lastTop]);
    }

    /**
     * The contents of the column containing the last played chip.
     */
    private String vertical() {
        StringBuilder sb = new StringBuilder(this.height);
        for (int h = 0; h < this.height; h++) {
            sb.append(this.grid[h][this.lastCol]);
        }
        return sb.toString();
    }

    /**
     * The contents of the "/" diagonal containing the last played chip
     * (coordinates have a constant sum).
     */
    private String slashDiagonal() {
        StringBuilder sb = new StringBuilder(this.height);
        for (int h = 0; h < this.height; h++) {
            int w = this.lastCol + this.lastTop - h;
            if (0 <= w && w < this.width) {
                sb.append(this.grid[h][w]);
            }
        }
        return sb.toString();
    }

    /**
     * The contents of the "\" diagonal containing the last played chip
     * (coordinates have a constant difference).
     */
    private String backslashDiagonal() {
        StringBuilder sb = new StringBuilder(this.height);
        for (int h = 0; h < this.height; h++) {
            int w = this.lastCol - this.lastTop + h;
            if (0 <= w && w < this.width) {
                sb.append(this.grid[h][w]);
            }
        }
        return sb.toString();
    }

    private static boolean contains(String haystack, String needle) {
        return haystack.indexOf(needle) >= 0;
    }

    public static void main(String[] args) {
        try (Scanner input = new Scanner(System.in)) {
            int height = 6, width = 8, moves = height * width;
            ConnectFour board = new ConnectFour(width, height);
            System.out.println("Use 0-" + (width - 1) + " to choose a column.");
            System.out.println(board);

            for (int player = 0; moves-- > 0; player = 1 - player) {
                char symbol = players[player];
                board.chooseAndDrop(symbol, input);
                System.out.println(board);
                if (board.isWinningPlay()) {
                    System.out.println("Player " + symbol + " wins!");
                    return;
                }
            }
            System.out.println("Game over, no winner.");
        }
    }
}
Source Link
200_success
  • 145.7k
  • 22
  • 191
  • 481

User Experience

It would be nice if you printed the column numbers along with the board.

Instead of "Player 1" and "Player 2", why not just call them "Player X" and "Player O"? Then there is no confusion as to which player has which symbol.

Bugs

Your WIDTH and HEIGHT both happen to be 6. However, if you change, for example, WIDTH to 8, you'll see that you have confused the two.

A player who enters an invalid move forfeits a turn and gets no opportunity to correct the input. That seems rather unfair. Furthermore, "That's not a valid column" should be more helpful: error messages should not only state what went wrong, but provide guidance on what inputs are allowable.

CheckXDiagonalForward() and CheckXDiagonalBack() both do exactly the same thing: they detect wins along ╲, but not ╱. As @h.j.k. points out, there really shouldn't be that much repetition among your functions.

Implementation

By convention, Java method names start with lowerCase().

The names CreateBoard() and PrintBoard() strongly suggest that you should have an object representing a board. Then you would have a constructor and a toString() method.

When checking for a winner, you're examining the entire board. You only need to look at the neighbours of the last chip played, in 7 directions.

The loops are cumbersome. Variable names like counter and flag are not useful at all. Instead of counter, pick something meaningful like row or streakLength. Instead of flag variables, use proper flow of control techniques like continue, break, and return.

Suggested solution

import java.util.Arrays;
import java.util.Scanner;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public class ConnectFour {
    private static final char[] players = new char[] { 'X', 'O' };

    private final int width, height;
    private final char[][] grid;

    public ConnectFour(int width, int height) {
        this.width = width;
        this.height = height;
        this.grid = new char[height][];
        for (int h = 0; h < height; h++) {
            Arrays.fill(this.grid[h] = new char[width], '.');
        }
    }

    public String toString() {
        return IntStream.range(0, this.width)
                        .mapToObj(Integer::toString)
                        .collect(Collectors.joining()) + "\n" +
               Arrays.stream(this.grid)
                     .map(String::new)
                     .collect(Collectors.joining("\n"));
    }

    /**
     * Prompts the user for a column, repeating until a valid
     * choice is made.
     *
     * @return the chosen column
     */
    public int chooseAndDrop(char symbol, Scanner input) {
        do {
            System.out.print("\nPlayer " + symbol + " turn: ");
            int col = input.nextInt();

            if (! (0 <= col && col < this.width)) {
                System.out.println("Column must be between 0 and " +
                                   (this.width - 1));
                continue;
            }
            for (int h = this.height - 1; h >= 0; h--) {
                if (this.grid[h][col] == '.') {
                    this.grid[h][col] = symbol;
                    return col;
                }
            }

            System.out.println("Column " + col + " is full.");
        } while (true);
    }

    private static final int[][] DIRECTIONS = new int[][] {
        new int[] { -1, -1 },
        new int[] { -1,  0 },
        new int[] { -1, +1 },
        new int[] {  0, +1 },
        new int[] { +1, +1 },
        new int[] { +1,  0 },
        new int[] { +1, -1 }
    };

    /**
     * Detects whether the last chip played in the specified
     * column was a winning move.
     */
    public boolean isWinningPlay(int col) {
        for (int top = 0; top < this.height; top++) {
            if (this.grid[top][col] != '.') {
                for (int d = 0; d < DIRECTIONS.length; d++) {
                    int dw = DIRECTIONS[d][0],
                        dh = DIRECTIONS[d][1];
                    if (this.isWinningPlay(col, top, dw, dh)) {
                        return true;
                    }
                }
                return false;
            }
        }
        assert false;
        return false;
    }

    private boolean isWinningPlay(int col, int top, int dw, int dh) {
        char symbol = this.grid[top][col];
        for (int consec = 1; consec < 4; consec++) {
            int w = col + consec * dw,
                h = top + consec * dh;
            if ( !(0 <= w && w < this.width) ||
                 !(0 <= h && h < this.height) ||
                 this.grid[h][w] != symbol ) {
                return false;
            }
        }
        return true;
    }

    public static void main(String[] args) {
        try (Scanner input = new Scanner(System.in)) {
            int height = 6, width = 8;
            ConnectFour board = new ConnectFour(width, height);
            System.out.println("Use 0-" + (width - 1) + " to choose a column.");
            System.out.println(board);

            for (int player = 0; ; player = 1 - player) {
                char symbol = players[player];
                int chosenColumn = board.chooseAndDrop(symbol, input);
                System.out.println(board);
                if (board.isWinningPlay(chosenColumn)) {
                    System.out.println("Player " + symbol + " wins!");
                    break;
                }
            }
        }
    }
}