Connect Four game with minimax AI











up vote
5
down vote

favorite












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);
}
}









share|improve this question
























  • trying to figure out how your "lineOfFour" works...can you provide some comments aoround that?
    – user5096910
    Dec 11 at 19:47















up vote
5
down vote

favorite












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);
}
}









share|improve this question
























  • trying to figure out how your "lineOfFour" works...can you provide some comments aoround that?
    – user5096910
    Dec 11 at 19:47













up vote
5
down vote

favorite









up vote
5
down vote

favorite











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);
}
}









share|improve this question















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);
}
}






java ai connect-four






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Dec 22 '16 at 12:27









200_success

128k15149412




128k15149412










asked Dec 21 '16 at 22:37









Micheal C.

2815




2815












  • trying to figure out how your "lineOfFour" works...can you provide some comments aoround that?
    – user5096910
    Dec 11 at 19:47


















  • trying to figure out how your "lineOfFour" works...can you provide some comments aoround that?
    – user5096910
    Dec 11 at 19:47
















trying to figure out how your "lineOfFour" works...can you provide some comments aoround that?
– user5096910
Dec 11 at 19:47




trying to figure out how your "lineOfFour" works...can you provide some comments aoround that?
– user5096910
Dec 11 at 19:47










2 Answers
2






active

oldest

votes

















up vote
3
down vote



accepted










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.






share|improve this answer




























    up vote
    6
    down vote













    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.






    share|improve this answer























      Your Answer





      StackExchange.ifUsing("editor", function () {
      return StackExchange.using("mathjaxEditing", function () {
      StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
      StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
      });
      });
      }, "mathjax-editing");

      StackExchange.ifUsing("editor", function () {
      StackExchange.using("externalEditor", function () {
      StackExchange.using("snippets", function () {
      StackExchange.snippets.init();
      });
      });
      }, "code-snippets");

      StackExchange.ready(function() {
      var channelOptions = {
      tags: "".split(" "),
      id: "196"
      };
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function() {
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled) {
      StackExchange.using("snippets", function() {
      createEditor();
      });
      }
      else {
      createEditor();
      }
      });

      function createEditor() {
      StackExchange.prepareEditor({
      heartbeatType: 'answer',
      autoActivateHeartbeat: false,
      convertImagesToLinks: false,
      noModals: true,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      imageUploader: {
      brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
      contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
      allowUrls: true
      },
      onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      });


      }
      });














      draft saved

      draft discarded


















      StackExchange.ready(
      function () {
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f150541%2fconnect-four-game-with-minimax-ai%23new-answer', 'question_page');
      }
      );

      Post as a guest















      Required, but never shown

























      2 Answers
      2






      active

      oldest

      votes








      2 Answers
      2






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes








      up vote
      3
      down vote



      accepted










      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.






      share|improve this answer

























        up vote
        3
        down vote



        accepted










        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.






        share|improve this answer























          up vote
          3
          down vote



          accepted







          up vote
          3
          down vote



          accepted






          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.






          share|improve this answer












          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.







          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered Dec 22 '16 at 10:24









          mtj

          2,869313




          2,869313
























              up vote
              6
              down vote













              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.






              share|improve this answer



























                up vote
                6
                down vote













                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.






                share|improve this answer

























                  up vote
                  6
                  down vote










                  up vote
                  6
                  down vote









                  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.






                  share|improve this answer














                  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.







                  share|improve this answer














                  share|improve this answer



                  share|improve this answer








                  edited Dec 21 '16 at 23:02

























                  answered Dec 21 '16 at 22:54









                  Roland Illig

                  10.8k11844




                  10.8k11844






























                      draft saved

                      draft discarded




















































                      Thanks for contributing an answer to Code Review Stack Exchange!


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid



                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.


                      Use MathJax to format equations. MathJax reference.


                      To learn more, see our tips on writing great answers.





                      Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


                      Please pay close attention to the following guidance:


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid



                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.


                      To learn more, see our tips on writing great answers.




                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function () {
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f150541%2fconnect-four-game-with-minimax-ai%23new-answer', 'question_page');
                      }
                      );

                      Post as a guest















                      Required, but never shown





















































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown

































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown







                      Popular posts from this blog

                      Сан-Квентин

                      8-я гвардейская общевойсковая армия

                      Алькесар