How to run two thread periodically?

Question

I am asked to make a Player class ( that implements runnable ) in which I implement a run method which generates a random number and waits for the method getchoice and a getchoice method which return the generated number and notifies run . then a RockPaperScissors class( that implements Runnable) which contains method run that containt two threads containing each a player object , these two threads should play against each other 1000 times , then they should be interrupted and then it will be displayed how many times each player won . . my problem is , when my code starts to run , at the beginning it is perfect , but at a random round it starts to just play many times player 1 before going to player 2 which defeats the purpose of the game : here is the code :

public class RockPaperScissors implements Runnable {
int result1 ;
int result2 ;
int result3 ;

private final Object lock = new Object();

public void run(){


    synchronized(lock){
        Player a = new Player() ;   
        Player b = new Player() ;
    Thread a1 = new Thread(a) ;
    Thread b1= new Thread (b) ;
        a1.start(); 
        b1.start(); 
        int choice1 = -100 ;
    int choice2 = -1066 ;
  for (int i = 0 ; i < 1000 ; i++){



    try {

        choice1 = a.getChoice();    
    } catch (InterruptedException e1) {

    }


    try {

        choice2 = b.getChoice();

    } catch (InterruptedException e) {

    }
    if (choice1 == 1 && choice2==0)
        result2++;
    else if (choice2 == 1 && choice1==0)
        result1++;
    else if (choice1 == 1 && choice2==1)
        result3++ ;
    else if (choice1 == 1 && choice2==2)
        result1++ ;
    else if (choice1 == 2 && choice2==1)
        result2++ ;
    else if (choice1 == 0 && choice2==2)
        result1++ ;
    else if (choice1 == 2 && choice2==0)
        result2++ ;
    else if (choice1 == 2 && choice2==2)
        result3++ ;
    else if (choice1 == 0 && choice2==0)
        result3++ ;


    }

and this is class Player :

public class Player implements Runnable {
private final Object lockvalue = new Object();
private int a;

public void run() {
    synchronized (lockvalue) {
        for (int counter = 0; counter < 1000; counter++) {

            java.util.Random b = new java.util.Random();
            a = b.nextInt(3);
            System.out.println(counter);
            try {
                lockvalue.wait();
            } catch (InterruptedException e) {
                System.out.println("Thread player was interrupted");

            }

        }

    }
}

public int getChoice() throws InterruptedException {
    synchronized (lockvalue) {
        lockvalue.notify();
        return a;

    }

}

}

if my programm runs perfectly the counter display should always have the number starting from 0 to 1000 duplicated one after the other , but here it starts like that but then it gets messed up and it never reaches 1000 , it stops sometimes at 700 sometimes at 800 . all I am allowed to used is notify() , notifyAll() , wait() , start() , interrupt() and join() .

your help would be greatly appreciated . Thanks

Answer 1

I would consider implementing your logic using a semaphore or a CountDownLatch for better synchronization

http://tutorials.jenkov.com/java-util-concurrent/semaphore.html

http://tutorials.jenkov.com/java-util-concurrent/countdownlatch.html

Answer 2

Your implementation and approach show that you don't understand concurrency, how it works and when should be applied.

I recommend you to read corresponding chapter (Concurrency) in Bruce Eckel's "Thinking in Java" - http://www.mindview.net/Books/TIJ/

To make your code work You have to add one more wait-notify before return in Player.getChoise()

Here is fixed version:

RockPaperScissors.java

package game;

public class RockPaperScissors
{
  static int player1wins = 0;
  static int player2wins = 0;
  static int draw = 0;

  public static void main(String[] args) throws InterruptedException
  {
    int cycles = 1000;
    Player player1 = new Player("Player-1", cycles);
    Player player2 = new Player("Player-2", cycles);

    new Thread(player1).start();
    new Thread(player2).start();


    for (int i = 0; i < cycles; i++)
    {
      Choice choice1;
      Choice choice2;

      choice1 = player1.getChoice();
      System.out.println("Value 1 is definitely generated");

      choice2 = player2.getChoice();
      System.out.println("Value 2 is definitely generated");

      System.out.printf("\n%3d\nPlayer1 - %8s\nPlayer2 - %8s\n", i, choice1.name(), choice2.name());

      if (choice1 == choice2)
      {
        draw++;
        System.out.println("Draw!");
      }
      else if (choice1 == Choice.ROCK)
      {
        if (choice2 == Choice.PAPER)
        {
          player2wins++;
          System.out.println("2 wins!");
        }
        else
        {
          player1wins++;
          System.out.println("1 wins!");
        }
      }
      else if (choice1 == Choice.PAPER)
      {
        if (choice2 == Choice.SCISSORS)
        {
          player2wins++;
          System.out.println("2 wins!");
        }
        else
        {
          player1wins++;
          System.out.println("1 wins!");
        }
      }
      else if (choice1 == Choice.SCISSORS)
      {
        if (choice2 == Choice.ROCK)
        {
          player2wins++;
          System.out.println("2 wins!");
        }
        else
        {
          player1wins++;
          System.out.println("1 wins!");
        }
      }
    }
    System.out.printf("Player 1 wins - %3d times;\n" +
        "Player 2 wins - %3d times;\n" +
        "Draw result   - %3d times\n\n", player1wins, player2wins, draw);

    System.out.printf("Player-1 cycles left = %d\n" +
        "Player-2 cycles left = %d\n", player1.getCounter(), player2.getCounter());
  }
}

Player.java

package game;

import java.util.Random;

public class Player implements Runnable
{
  private Random random = new Random();
  private int value;
  private int counter;
  private String name;

  public Player(String name, int cycles)
  {
    this.name = name;
    this.counter = cycles;
  }

  public synchronized void run()
  {
    while (true)
    {
      try
      {
        wait();
      }
      catch (InterruptedException e)
      {
        e.printStackTrace();
      }

      value = random.nextInt(3);
      System.out.println(name + " ... Value was generated = " + value);
      notify();

      // Otherwise your thread will never stop!
      counter--;
      if (counter <= 0)
      {
        System.out.println(name + " ... Limit of operations is exceeded.");
        break;
      }
    }
  }

  public synchronized Choice getChoice() throws InterruptedException
  {
    System.out.println(name + " ... now can generate value");
    notify();
    System.out.println(name + " ... wait until value is generated");
    wait();
    Choice choice = Choice.values()[value];
    System.out.println(name + " ... returning generated value: " + value);
    return choice;
  }

  public int getCounter()
  {
    return counter;
  }
}

Choise.java

package game;

public enum Choice
{
  ROCK,
  PAPER,
  SCISSORS;
}