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);
}
}
java ai connect-four
add a comment |
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);
}
}
java ai connect-four
trying to figure out how your "lineOfFour" works...can you provide some comments aoround that?
– user5096910
Dec 11 at 19:47
add a comment |
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);
}
}
java ai connect-four
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
java ai connect-four
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
add a comment |
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
add a comment |
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.
add a comment |
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.
add a comment |
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
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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.
add a comment |
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.
add a comment |
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.
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.
answered Dec 22 '16 at 10:24
mtj
2,869313
2,869313
add a comment |
add a comment |
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.
add a comment |
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.
add a comment |
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.
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.
edited Dec 21 '16 at 23:02
answered Dec 21 '16 at 22:54
Roland Illig
10.8k11844
10.8k11844
add a comment |
add a comment |
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.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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
trying to figure out how your "lineOfFour" works...can you provide some comments aoround that?
– user5096910
Dec 11 at 19:47