Your code could be improved in several ways, but with a few little hacks it can work:
class Dice
{
int diceValue;
public Dice()
{
this.diceValue=0;
}
}
public class DiceGame implements Runnable
{
Dice d;
public DiceGame()
{
this.d=new Dice();
}
@Override
public void run()
{
if(Thread.currentThread().getName().equals("Player 1"))
{
final Random rg=new Random();
for(int i=0;i<6;i++)
{
synchronized(d)
{
d.diceValue=rg.nextInt(6);
System.out.println(Thread.currentThread().getName()+" dice Value is "+d.diceValue);
d.notifyAll();
try
{
d.wait();
}
catch (final InterruptedException e)
{
// TODO Auto-generated catch block
e.printStackTrace();
}
}
}
}
else if(Thread.currentThread().getName().equals("Player 2"))
{
final Random rg=new Random();
for(int i=0;i<6;i++)
{
synchronized(d)
{
try
{
d.wait();
}
catch (final InterruptedException e)
{
e.printStackTrace();
}
d.diceValue=rg.nextInt(6);
System.out.println(Thread.currentThread().getName()+" dice Value is "+d.diceValue);
d.notifyAll();
}
}
}
}
public static void main(final String []args) throws InterruptedException
{
final DiceGame dg=new DiceGame();
final Thread tr1=new Thread(dg);
final Thread tr2=new Thread(dg);
tr1.setName("Player 1");
tr2.setName("Player 2");
tr2.start();
Thread.sleep(100);
tr1.start();
}
}
I got no illegalMonitorstate exception, but the first thread get locked up forever. Basically the problem is that the first thread rolls the dice, and calls d.notifyAll before actually the 2nd thread could start and waiting for the dice. This is naively solved by first starting thread 2 then waiting a bit and start thread 1.
You might also consider:
- using the Java convention for braces { }
- rg.nextInt gives values between 0..5, not 1..6
- it is bad idea to make the thread code work differently depending on the name of the thread. In OOP different behavior is expressed with descendant classes instead.
I guess you're wishing for an universal solution for rolling the dice with multiple players. This problem is not necessarily a problem which requires concurrent programming, since rolling the dice goes serially amongst players. You can of course have the players as Threads, but only one Thread will be active at any point of time. In case of using Threads, you shall implement your own scheduling logic which ensures consistent one-after-another scheduling of threads. Using a monitor (e.g. synchornize(d)) does not offer any guarantee of the ordering, it is only built to guarantee that up to one thread can access to the dice at any point in time.
A solution with any number of players but no threads (this is not a concurrency problem after all) shows this behavior:
import java.util.Random;
class Dice {
private final Random rg=new Random();
private int diceValue=1;
public void roll() {
diceValue=rg.nextInt(6)+1;
}
@Override
public String toString() {
return "value="+diceValue;
}
}
public class Player extends Thread {
Dice dice;
int rollsLeft=6;
public Player(final Dice dice) {
this.dice=dice;
}
@Override
public void run() {
while (rollsLeft>0) {
synchronized(dice) {
// dice obtained
dice.roll();
System.out.println(Thread.currentThread().getName()+" rolled "+dice);
}
// dice released
rollsLeft--;
// just wait a little to make it slower and let other threads to join
try {
Thread.sleep(100);
} catch (final InterruptedException e) {
// ignore
}
}
}
public static void main(final String []args) throws InterruptedException {
final Dice dice=new Dice();
final Player player1=new Player(dice);
final Player player2=new Player(dice);
player1.start();
player2.start();
}
}
Which gives:
Thread-0 rolled value=1
Thread-1 rolled value=6
Thread-0 rolled value=2
Thread-0 rolled value=4
Thread-1 rolled value=2
etc...
As you can see, the order (i.e. player1, player2, player1, player2) is not guaranteed.
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
class Dice {
private final Random rg=new Random();
private int diceValue=1;
public void roll() {
diceValue=rg.nextInt(6)+1;
}
@Override
public String toString() {
return "value="+diceValue;
}
}
public class Player {
Dice dice;
String player;
public Player(final Dice dice,final String player) {
this.dice=dice;
this.player=player;
}
public void roll() {
dice.roll();
System.out.println(player+" rolled "+dice);
}
public static void main(final String []args) throws InterruptedException {
final Dice dice=new Dice();
final List<Player> players=new ArrayList<Player>();
players.add(new Player(dice,"Ann"));
players.add(new Player(dice,"Ben"));
players.add(new Player(dice,"Cecil"));
players.add(new Player(dice,"Denise"));
for (int rounds=0;rounds<6;rounds++) {
System.out.println("---");
for (final Player player:players) {
player.roll();
}
}
}
}
Which gives you the expected output, i.e. Ann, Ben, Cecil, Denise has 6 rounds of rolling the dice.
