Question

I asked this question yesterday and have attempted to implement the top answer I received. So from the code from yesterday I have tried adding synchronized to my methods and using wait() and notifyAll(). I have been looking up a bunch of examples, and reading the documentation but I'm definitely not doing it right.

Basically, once a touchEvent happens my ButtonListener stops all of my other code from executing, and only performs the code contained within the ButtonListener. This only happens on one of my computers though, my laptop runs the code the way I expected it to, my desktop gets stuck in the ButtonListener. This was a school assignment I turned in a couple weeks ago and initially received a 70%, but I explained to my teacher that it works on some computers and he ran it on his office desktop, and viola, it worked and I got a 100%. Of course I want to figure out what the problem is though so that is why I'm still working on this.

Here are the offending code snippets:

    public synchronized void playOneTurn(int player)
        throws InterruptedException {
    waiting = true;
    while (waiting) {
        try {
            System.out.println("playOneTurn before wait.");
            wait();
        } catch (InterruptedException e) {
            // e.printStackTrace();
        }
    }
    System.out.println("playOneTurn AFTER wait.");
    }

My method here, playOneTurn, is the last bit of code that runs before the first touch event. Previously, as you can tell by looking at my question linked at the top, I used a simple while loop that waited for my ButtonListener to flip a boolean, waiting. Above is my attempt to use synchronized.

    class ButtonListener implements ActionListener {
    @Override
    public synchronized void actionPerformed(ActionEvent e) {
        System.out.println("Entering Action Performed");
        for (i = 0; i < size; i++) {
            for (j = 0; j < size; j++) {
                if (e.getSource() == cells[i][j]) {
                    if (model[i][j] == 'e') {
                        cells[i][j].setText("");
                        currentSpot[0] = i;
                        currentSpot[1] = j;
                        if (count % 2 == 0) {
                            cells[i][j].setBackground(Color.GREEN);
                            cells[i][j].setIcon(X_MARK);
                            model[i][j] = 'x';
                            count++;
                            waiting = false;
                            System.out.println("Boolean hit");
                            notifyAll();
                        } else {
                            cells[i][j].setBackground(Color.CYAN);
                            cells[i][j].setIcon(O_MARK);
                            model[i][j] = 'o';
                            count++;
                            waiting = false;
                            System.out.println("Boolean hit");
                            notifyAll();
                        }

                    } else {
                        System.out
                                .println("Hey, you can't move there silly!");
                    }
                }
            }
        }
    }
    }

Here is my ButtonListener where my program just chills in and stops doing anything else. Please disregard my random printlns everywhere, I was just getting aggravated and trying to find out what this thing is doing.

I also had to add this try/catch block to my controller class:

        while (!game.haveWinner() && !game.isFull()) {
        player = (player + 1) % 2;
        try {
            game.playOneTurn(player);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
             e.printStackTrace();
        }
        System.out.println("While loop looped");
    }

I have been trying every way I can think of to correctly implement synchronized but I'm obviously doing something wrong.

Here is a Dropbox link to the entire program if it would help.

Was it helpful?

Solution

This:

while (!game.haveWinner() && !game.isFull()) {

Is going to tie up the Swing event thread and freeze your entire application .... don't do it.

Your code looks like you're trying to hack a linear console program into a Swing GUI, and that never works because their program flow and logic are completely different. The solution is to change your logic to more event-driven.

You presumably have a game loop somewhere, perhaps using a Swing Timer ... so check with each iteration of the loop for winner or for is full.

You need to get all waiting, all synchronized, all notifies out of your program. Instead make a button press change it's state.


Edit
I was playing around with your code, and came up with something like this. Note that synchronization is not my strong suite, so take this with a grain of salt, but my key goal is to make sure that Swing GUI creation code and state changing code is called only on the Swing event thread, and that any other code, especially code that requires synchronization with other threads, is not called on the Swing event thread.

import java.awt.BorderLayout;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Scanner;
import javax.swing.*;

public class TicTacToeApp {
   public static void main(String[] args) {
      TicTacToeGame game;
      int size, need, player = 1;
      String[] names = new String[2];
      Scanner kbd = new Scanner(System.in);

      // TODO: uncomment in running code
      // System.out.print("Enter Player 1's name:  ");
      // names[0] = kbd.nextLine();
      // System.out.print("Enter Player 2's name:  ");
      // names[1] = kbd.nextLine();
      //
      // System.out.print("Enter the TIC-TAC-TOE grid size:  ");
      // size = kbd.nextInt();
      // System.out.print("Enter how many in a row you need to win:  ");
      // need = kbd.nextInt();
      // System.out.println();

      // TODO: For test purposes only. Delete in running code
      size = 3;
      need = 3;
      names[0] = "Foo";
      names[1] = "Bar";

      game = new TicTacToeGame(size, need, names);

      while (!game.haveWinner() && !game.isFull()) {
         player = (player + 1) % 2;
         try {
            game.playOneTurn(player);
         } catch (InterruptedException e) {
            e.printStackTrace();
         }
         System.out.println("While loop looped");
      }

      if (game.haveWinner())
         System.out.println(names[player] + " is the winner!");
      else
         System.out.println("It's a TIE!");

      System.out.println("\nBye!");

   }
}

@SuppressWarnings("serial")
class TicTacToeGame extends JPanel {
   private static final Object LOCK = new Object();
   private volatile int player = 0;
   private int size;
   private int need;
   private String[] names;
   private JLabel nameLabel = new JLabel();
   // private JButton testButton = new JButton();
   private JButton[][] buttonGrid;
   private volatile boolean waiting = false;

   public TicTacToeGame(int size, int need, String[] names) {
      this.size = size;
      this.need = need;
      this.names = names;

      nameLabel.setText(names[0]);

      JPanel topPanel = new JPanel();
      topPanel.add(new JLabel("Player:"));
      topPanel.add(nameLabel);

      buttonGrid = new JButton[size][size];
      ButtonListener actionListener = new ButtonListener(this);
      JPanel middlePanel = new JPanel(new GridLayout(size, size));
      for (int row = 0; row < size; row++) {
         for (int col = 0; col < size; col++) {
            JButton button = new JButton("   ");
            middlePanel.add(button);
            buttonGrid[row][col] = button;
            button.addActionListener(actionListener);
         }
      }

      setLayout(new BorderLayout());
      add(topPanel, BorderLayout.NORTH);
      add(middlePanel, BorderLayout.CENTER);

      // run GUI on Swing event thread
      SwingUtilities.invokeLater(new Runnable() {
         public void run() {
            JFrame frame = new JFrame("Test");
            frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
            frame.getContentPane().add(TicTacToeGame.this);
            frame.pack();
            frame.setLocationRelativeTo(null);
            frame.setVisible(true);
         }
      });
   }

   public int getPlayer() {
      return player;
   }

   public synchronized void playOneTurn(final int player)
         throws InterruptedException {
      this.player = player;
      System.out.printf("Player %d before wait%n", player);
      SwingUtilities.invokeLater(new Runnable() {
         public void run() {
            nameLabel.setText(names[player]);
         }
      });
      synchronized (LOCK) {
         waiting = true;
         while (waiting) {
            LOCK.wait();
         }
      }
   }

   public boolean isFull() {
      for (int row = 0; row < size; row++) {
         for (int col = 0; col < size; col++) {
            if (buttonGrid[row][col].isEnabled()) {
               return false;
            }
         }
      }
      return true;
   }

   public boolean haveWinner() {
      // TODO finish this
      return false;
   }

   public void doNotification() {
      new Thread(new Runnable() {
         public void run() {
            synchronized (LOCK) {
               waiting = false;
               LOCK.notifyAll();
            }
         }
      }).start();
   }

   public void tttButtonPressed(ActionEvent e) {
      AbstractButton source = (AbstractButton) e.getSource();
      for (int r = 0; r < size; r++) {
         for (int c = 0; c < size; c++) {
            if (buttonGrid[r][c] == source) {
               String text = player == 0 ? "X" : "0";
               source.setText(text);
               source.setEnabled(false);
            }
         }
      }
      doNotification();
   }

}

class ButtonListener implements ActionListener {
   private TicTacToeGame ticTacToeGame;

   public ButtonListener(TicTacToeGame ticTacToeGame) {
      this.ticTacToeGame = ticTacToeGame;
   }

   public void actionPerformed(ActionEvent e) {
      ticTacToeGame.tttButtonPressed(e);
   };
}

OTHER TIPS

Oh OK, I found the bug.

public class TicTacToeGame extends JFrame {
    public synchronized void playOneTurn(int player)
            throws InterruptedException {
        wait();
    }

    class ButtonListener implements ActionListener {
        @Override
        public synchronized void actionPerformed(ActionEvent e) {
            notifyAll();
        }
    }
}

I reduced the code so it's more obvious. playOneTurn is synchronized and waits on the TicTacToeGame instance but actionPerformed is synchronized and notifies on the ButtonListener instance.

A fix would be something like this (since ButtonListener is an inner class):

@Override
public void actionPerformed(ActionEvent e) {
    synchronized (TicTacToeGame.this) {
        TicTacToeGame.this.notifyAll();
    }
}

Or create a separate object just to be a monitor.

But as I said before (and @HovercraftFullOfEels also seems to be saying), you could do to simply remove the while loop from the program and just use the event as a single entry point.

Also: don't forget to create your GUI on the EDT with invokeLater.

When code of the ActionListner more time consuming in execution, it halts all other part of the program from running.

So, create a new thread when the control enters the ActionListner. so it leverage the main thread to run other code while the thread you created runs your ActionListner part of the code.

Example:

    void actionPerformed(ActionEvent e) {

    Thread t = new Thread(new runnable()) {

        public void run() {

            System.out.println("Entering Action Performed");

            for (i = 0; i < size; i++) {

                for (j = 0; j < size; j++) {

                    if (e.getSource() == cells[i][j]) {

                        if (model[i][j] == 'e') {

                            cells[i][j].setText("");

                            currentSpot[0] = i;

                            currentSpot[1] = j;

                            if (count % 2 == 0) {

                                cells[i][j].setBackground(Color.GREEN);

                                cells[i][j].setIcon(X_MARK);

                                model[i][j] = 'x';

                                count++;

                                waiting = false;
                                System.out.println("Boolean hit");

                                notifyAll();

                            } else {

                                cells[i][j].setBackground(Color.CYAN);

                                cells[i][j].setIcon(O_MARK);

                                model[i][j] = 'o';

                                count++;

                                waiting = false;

                                System.out.println("Boolean hit");

                                notifyAll();

                            }

                        } else {
                            System.out
                                    .println("Hey, you can't move there silly!");

                        }
                    }
                }
            }
        }
    };// runnable ends here

}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top