Cfrtjryk

I have created a connect four game between a bot and a player. It was quite a bit of a challenge since I wasn't too fond of the minmax algorithm until now. I know there's always room for improvement and I am wondering if someone can tell me if my code is up to standards, i.e. better solutions etc. Please feel free to scrutinize my code.

package connect4;



import java.util.Arrays;

import java.util.Scanner;

import java.util.stream.Collectors;

import java.util.stream.IntStream;



import javax.swing.*;

import java.awt.*;

import java.awt.event.*;

import java.io.*;

import java.util.*;



enum Piece

{

    Red,

    Blue,

    None

}

class Board extends JButton

{

    public int i, j;

    public Piece piece = Piece.None;



    public Board(int i, int j)

    {

        this.i = i;

        this.j = j;

        setOpaque(true);

        setColor();

    }

    public void setPiece(Piece piece)

    {

        this.piece = piece;

        setColor();

    }

    public void setColor()

    {

        switch(piece)

        {

            case Red:

                setBackground(Color.red);

                break;

            case Blue:

                setBackground(Color.blue);

                break;

            case None:

                setBackground(Color.white);

                break;

        }

    }

}



class Tree  // this is the minmax algorithm

{

    public int value;

    Board Boards; // this is the board

    private ArrayList<Integer> bestMoves;

    Board prev = null;

    int depth;

    static int maxDepth = 4;  // this is the max depth im going down



    public Tree(Board Boards, int depth)

    {

        this.Boards = Boards;

        this.bestMoves = new ArrayList<Integer>();

        this.depth = depth;

        this.value = getValue();



        if(depth < maxDepth && this.value < 100 && this.value > -100 )

        {

            ArrayList<Integer> possibilities = new ArrayList<Integer>();

            for(int i = 0; i < 7; i++)

                if(Boards[i][0].piece == Piece.None)

                    possibilities.add(i);



            for(int i = 0; i < possibilities.size(); i++)

            {

                insertTo(Boards[possibilities.get(i)][0]);

                Tree child = new Tree(Boards, depth+1);

                prev.setPiece(Piece.None);



                if(i == 0)

                {

                    bestMoves.add(possibilities.get(i));

                    value = child.value;

                }

                else if(depth % 2 == 0)

                {

                    if(value < child.value)

                    {

                        bestMoves.clear();

                        bestMoves.add(possibilities.get(i));

                        this.value = child.value;

                    }

                    else if(value == child.value)

                        bestMoves.add(possibilities.get(i));

                }

                else if(depth % 2 == 1)

                {

                    if(value > child.value)

                    {

                        bestMoves.clear();

                        bestMoves.add(possibilities.get(i));

                        this.value = child.value;

                    }

                    else if(value == child.value)

                        bestMoves.add(possibilities.get(i));

                }

            }

        }

        else

        {

            this.value = getValue();

        }

    }



    void printBoards() //printboard

    {

        for(int j = 0; j < 6; j++)

        {

            for(int i = 0; i < 7; i++)

            {

                switch(Boards[i][j].piece)

                {

                    case Blue: System.out.print("B"); break;

                    case Red: System.out.print("R"); break;

                    default: System.out.print("-"); break;

                }

            }

            System.out.println();

        }

    }



    void insertTo(Board Board)  // insert into board

    {

        if(Board.piece != Piece.None)

            return;



        int i = Board.i;

        int j = Board.j;



        while(j < Boards[0].length-1 && Boards[i][j+1].piece == Piece.None)

            j++;



        if(depth % 2 == 0)

            Boards[i][j].setPiece(Piece.Red);

        else

            Boards[i][j].setPiece(Piece.Blue);

        prev = Boards[i][j];

    }



    public int getX()  // get the player move 

    {

        int random = (int)(Math.random() * 100) % bestMoves.size();

        return bestMoves.get(random);

    }



    public int getValue() // get the value of each move

    {

        int value = 0;

        for(int j = 0; j < 6; j++)

        {

            for(int i = 0; i < 7; i++)

            {

                if(Boards[i][j].piece != Piece.None)

                {

                    if(Boards[i][j].piece == Piece.Red)

                    {

                        value += possibleConnections(i, j) * (maxDepth - this.depth);

                    }

                    else

                    {

                        value -= possibleConnections(i, j) * (maxDepth - this.depth);

                    }

                }

            }

        }

        return value;

    }



    public int possibleConnections(int i, int j)

    {

        int value = 0;

        value += lineOfFour(i, j, -1, -1);

        value += lineOfFour(i, j, -1, 0);

        value += lineOfFour(i, j, -1, 1);

        value += lineOfFour(i, j, 0, -1);

        value += lineOfFour(i, j, 0, 1);

        value += lineOfFour(i, j, 1, -1);

        value += lineOfFour(i, j, 1, 0);

        value += lineOfFour(i, j, 1, 1);



        return value;

    }



    public int lineOfFour(int x, int y, int i, int j)

    {

        int value = 1;

        Piece color = Boards[x][y].piece;



        for(int k = 1; k < 4; k++)

        {

            if(x+i*k < 0 || y+j*k < 0 || x+i*k >= Boards.length || y+j*k >= Boards[0].length)

                return 0;

            if(Boards[x+i*k][y+j*k].piece == color)

                value++;

            else if (Boards[x+i*k][y+j*k].piece != Piece.None)

                return 0;

            else

            {

                for(int l = y+j*k; l >= 0; l--)

                    if(Boards[x+i*k][l].piece == Piece.None)

                        value--;

            }

        }



        if(value == 4) return 100;

        if(value < 0) return 0;

        return value;

    }

}



public class ConnectFour extends JFrame implements ActionListener

{

    JLabel lblPlayer = new JLabel("Player: ");

    JLabel lblCurrentPlayer = new JLabel("Blue");

    JPanel pnlMenu = new JPanel();

    JPanel pnlBoards = new JPanel();

    JButton btnNewGame2 = new JButton("New Game");



    Board Boards = new Board[7][6];



    boolean winnerExists = false;

    int currentPlayer = 1;

    boolean AI;



    public ConnectFour(boolean AI)

    {

        super("Four In A Line");

        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);



        currentPlayer = (int)(Math.random()*2) + 1;



        this.AI = AI;



        btnNewGame2.addActionListener(this);

        switch(currentPlayer)

        {

            case 1:

                lblCurrentPlayer.setForeground(Color.blue);

                lblCurrentPlayer.setText("Blue");

                break;

            case 2:

                lblCurrentPlayer.setForeground(Color.red);

                lblCurrentPlayer.setText("Red");

                break;

        }

        pnlMenu.add(btnNewGame2);

        pnlMenu.add(lblPlayer);

        pnlMenu.add(lblCurrentPlayer);



        pnlBoards.setLayout(new GridLayout(6, 7));



        for(int j = 0; j < 6; j++)

            for(int i = 0; i < 7; i++)

            {

                Boards[i][j] = new Board(i, j);

                Boards[i][j].addActionListener(this);

                pnlBoards.add(Boards[i][j]);

            }



        add(pnlMenu, BorderLayout.NORTH);

        add(pnlBoards, BorderLayout.CENTER);    

        setSize(500, 500);

        setVisible(true);



        if(currentPlayer == 2 && AI) insertTo(minimax());

    }



    public void actionPerformed(ActionEvent ae)

    {



        if(ae.getSource() == btnNewGame2)

        {

            if(JOptionPane.showConfirmDialog(this, "Are you sure you want to quit?", "Confirmation", JOptionPane.YES_NO_OPTION) == 0)

            {

                dispose();

                new ConnectFour(true);

                return;

            }

        }

        else if(!winnerExists)

        {

            Board Board = (Board)ae.getSource();

            insertTo(Board);

        }

    }



    void insertTo(Board Board)

    {

        if(Board.piece != Piece.None)

            return;



        int i = Board.i;

        int j = Board.j;



        while(j < Boards[0].length-1 && Boards[i][j+1].piece == Piece.None)

            j++;



        switch(currentPlayer)

        {

            case 1:

                Boards[i][j].setPiece(Piece.Blue);

                break;

            case 2:

                Boards[i][j].setPiece(Piece.Red);

                break;

        }



        currentPlayer = (currentPlayer % 2) + 1;



        if(thereIsAWinner())

        {

            lblPlayer.setText("Winner: ");

            winnerExists = true;

        }

        else

        {

            switch(currentPlayer)

            {

                case 1:

                    lblCurrentPlayer.setForeground(Color.blue);

                    lblCurrentPlayer.setText("Blue");

                    break;

                case 2:

                    lblCurrentPlayer.setForeground(Color.red);

                    lblCurrentPlayer.setText("Red");

                    break;

            }



            if(currentPlayer == 2 && AI)

            {

                insertTo(minimax());

            }

        }

    }



    public boolean thereIsAWinner()

    {

        for(int j = 0; j < 6; j++)

        {

            for(int i = 0; i < 7; i++)

            {

                if(Boards[i][j].piece != Piece.None && connectsToFour(i, j))

                    return true;

            }

        }

        return false;

    }



    public boolean connectsToFour(int i, int j)

    {

        if(lineOfFour(i, j, -1, -1))

            return true;

        if(lineOfFour(i, j, -1, 0))

            return true;

        if(lineOfFour(i, j, -1, 1))

            return true;

        if(lineOfFour(i, j, 0, -1))

            return true;

        if(lineOfFour(i, j, 0, 1))

            return true;

        if(lineOfFour(i, j, 1, -1))

            return true;

        if(lineOfFour(i, j, 1, 0))

            return true;

        if(lineOfFour(i, j, 1, 1))

            return true;

        return false;

    }



    public boolean lineOfFour(int x, int y, int i, int j)

    {

        Piece color = Boards[x][y].piece;



        for(int k = 1; k < 4; k++)

        {

            if(x+i*k < 0 || y+j*k < 0 || x+i*k >= Boards.length || y+j*k >= Boards[0].length)

                return false;

            if(Boards[x+i*k][y+j*k].piece != color)

                return false;

        }

        return true;

    }



    public Board minimax()

    {

        Tree tree = new Tree(Boards, 0);

        return Boards[tree.getX()][0];

    }



    public static void main(String args)

    {

        new ConnectFour(false);

    }

}

A few thoughts in addition to Roland's feedback:

Enum as a collection of constants in Java has the possibility to carry all its attributes. Thus, I'd rather put the color into the enum than see a switch over the enum elements:

enum Piece {

    Red(Color.RED),

    Blue(Color.BLUE),

    None(Color.WHITE);



    public final Color pieceColor;  // attributes of the specific constant

    private Piece(Color theColor) {  // constructor to set these attributes

        this.pieceColor = theColor;

    }

}



...

// while we are at it: this name is ugly: reserve setXXX for setters, rename to somthing like "updateColor"

public void setColor() {

    setBackground(piece.pieceColor);

}

Adding the print-string as a second attribute is left as an excersise ;-)

Magic numbers: at some places, you already iterate to Boards.length or Boards[i].length. At other places, you use constant 6s and 7s. Replace all the constant numbers to references to the given array length, so that the only place to use concrete numbers is the creation of the Board.

Repeating calls: chains of calls that all look the same always make me cringe, in this case the method possibleConnections. I much prefer to put the variable parts in a constant definition and have a loop over that:

int offsets = new int { { -1, -1 }, { -1, 0 }, ... };

int value = Arrays.stream(offsets).mapToInt(of -> lineOfFour(i, j, of[0], of[1])).sum();

Constructors with side-effects: A constructor should normally just create the object. When you call "new ConnectFour(true);" you are not really interested in the object at all. So, I suggest to at least remove the setVisible call from the constructor and have this in the caller method.

First, you should ask yourself whether the names you chose are good. For example, a typical game of connect four takes place on a single Board, yet you defined an array of boards and call this array a board. This doesn't make sense.

A completely different topic is spelling rules. In Java, field and variable names start with a lowercase letter. You have Board Board sometimes, which confuses every reader.

You did not say what you mean by optimize. But no matter if it's for speed or for clarity, you should separate the complicated game logic from the user interface logic. Doing this enables you to test the game logic using unit tests.

If you strive for speed, you should remember the last position where a piece was dropped, since you only need to check at this particular position whether there is a new row of 4.

Please use a program that formats your code whenever you save it. This will make it look more consistent.

Your use of the random number generator is unfair, since it favors lower indexes. A quick fix would be (int)(Math.random() * bestMoves.size()), but that still looks complicated. A better way is to define a random number generator and ask it for integer numbers:

class MiniMax {

    Random rnd = new Random();



    int randomMove() {

        return bestMoves.get(rnd.nextInt(bestMoves.size());

    }

}

Note that I changed the class name from Tree to MiniMax. By choosing clear names you make the comments of the form // This is … superfluous.