2

Does any one have any idea why this while look ONLY exits IF theres a System.out.println()?

The very same code doesn't work if I comment out the println()

do{

    System.out.println("");

    if(hasWon()){
        InOut.out(output() + "\nYou Won");
        System.exit(0);
    }
} while (!hasWon()) 

The program is as follows

static final int gridSize = Integer.parseInt(InOut.in(1, "Enter grid size"));
static Tile[][] board = new Tile[gridSize][gridSize];
static int winCond = 1;
static GuiFrame f = new GuiFrame(gridSize);
static BtnPanel p = new BtnPanel(gridSize);
static JButton[][] btn = new JButton[gridSize][gridSize];

public static void main(String[] args) {

    //Creating objects
    for (int i = 0; i < gridSize; i++) {
        for (int z = 0; z < gridSize; z++) {
            board[i][z] = new Tile();
        }
    }
    GUI();

    while (!hasWon()) {

        System.out.println("");

        if(hasWon()){
            InOut.out(output() + "\nYou Won");
            System.exit(0);
        }
    }
}

public static boolean hasWon() {
    boolean hasWon = true;

    for (int i = 0; i < gridSize; i++) {
        for (int z = 0; z < gridSize; z++) {
            hasWon = hasWon && (board[i][z].getStatus() == winCond);
        }
    }

    return hasWon;
}

public static String output() {
    String message = "";

    for (int i = 0; i < gridSize; i++) {
        for (int z = 0; z < gridSize; z++) {
            message += board[i][z].getStatus() + " ";
        }
        message += "\n";
    }

    return message;
}

public static void GUI() {

    for (int i = 0; i < gridSize; i++) {
        for (int z = 0; z < gridSize; z++) {
            String btnValue = "";
            btnValue += board[i][z].getStatus();
            btn[i][z] = new JButton();
            btn[i][z].setText(btnValue);
            btn[i][z].addActionListener(f);
            p.add(btn[i][z]);
        }
    }

    f.add(p, BorderLayout.CENTER);

}

public static void modifyGUI(int i, int z){
    btn[i][z].setText(String.valueOf(board[i][z].getStatus()));
}

This is a puzzle game where the user clicks a tile and adjacent tiles change as well. However when the puzzle is completed, without a println() it does not show completion, with a println() it exits the loop and exits the program.

To re-emphasize, everything works fine if theres a println() inside the while loop. When i comment it out it doesnt work.

DarkDestry
  • 183
  • 1
  • 11
  • 2
    Please show a short but complete program demonstrating the problem. – Jon Skeet May 29 '14 at 17:29
  • 7
    Where do you switch the "won" flag from false to true? I don't see it. – duffymo May 29 '14 at 17:29
  • how do you know it doesn't work without the System.out.println? as far as I can say, hasWon() always returns false. it's a weird construction anyway. it would be a bit clearer if you just: while(!hasWon()){} print // you won – Stultuske May 29 '14 at 17:31
  • Infinite loop/NoLoop+Semicolon missing. – akash May 29 '14 at 17:32
  • 1
    Guys, why would you ask "how do you know it doesn't work without.."? He's given us the conditions of the situation, so there's no point in berating OP for the hasWon() code.. My guess is there's another thread going on somewhere, but it DOESN'T MATTER. The point is that it DOES work without the syso and DOESN'T work with it. – Christopher Wirt May 29 '14 at 17:33
  • I have updated the OP with the full code. If need the other classes ill include it. Initially the syso there to check if the variable: win changes. However when i removed it, it did not exit. @TAsk Done using NetBeans IDE. If theres an omitted semicolon its during copy and paste from there. Either way, it runs fine with syso and does not without. – DarkDestry May 29 '14 at 17:39
  • possible duplicate of [Loop doesn't see changed value without a print statement](http://stackoverflow.com/questions/25425130/loop-doesnt-see-changed-value-without-a-print-statement) – Boann Aug 22 '14 at 09:31
  • You might need to set `hasWon = true;` in between the `i` and `z` loops. That's assuming the inner `z` loop is checking for the actual win in a set selected by the outer `i` loop. – jww Aug 22 '14 at 10:32

3 Answers3

4

What you have is a tight CPU intensive loop that is repeatedly calling hasWon(). It also looks like some other thread is responsible for updating the board state that results in the "won" position.

It looks like problem is that the tight CPU loop is causing the other thread to starve, and inserting the println is sufficient to allow the thread scheduler to reschedule.

Another possible explanation is that you have a shared data structure that one thread is reading and another is updating ... without any synchronization. In theory, the fact that you are not synchronizing could mean that changes made by the updating thread are never seen by the reading thread, because the compiler has not seen the need to insert "barrier" instructions that will cause the relevant cache flushing, etc.


As @chrylis notes, busy looping like that is horribly inefficient. What you need to do is replace the busy looping:

  • A hacky solution is to put a Thread.sleep() call into the loop (instead of the println call).

  • A better solution is to use Object.wait() and Object.notify(). The main thread calls wait() on a mutex object after each check, and the thread that is doing the updating calls notify() on the same mutex each time it does an update that might result in a "won" position.

The second solution also deals with the synchronization issue. The wait() and notify() methods can only be performed while holding the mutex; i.e. in a synchronized block or method.


Here are some code snippets to show how wait / notify can be implemented.

    public static final Object mutex = new Object();
    public static boolean finished = false;

    // One thread
    synchronized (mutex) {
        while (!finished) {
             mutex.wait();
        }
    }

    // Another thread
    synchronized (mutex) {
        finished = true;
        mutex.notify();
    }

Obviously, the use of statics is just for illustrative purposes.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • So am i supposed to add the notify() at the actionPerformed class and the wait() replacing the loop? If so what do i reference as the "Object" – DarkDestry May 29 '14 at 17:55
0

The fact that it works now is an accident. You need to declare your variable volatile if you want to force the computer to recheck it on every pass through the loop; otherwise, it could just read the value once and never look at it again.

Additionally, having a busy-wait loop like that is horribly inefficient. Instead, if you want to exit on win, have the code that sets that flag in the first place call the shutdown code directly.

chrylis -cautiouslyoptimistic-
  • 75,269
  • 21
  • 115
  • 152
  • Not exactly. The variable will be read from the nearest cache possible (which is probably a cpu register). You need to make it volatile if you know someone else might be changing it from some other thread. I guess the System.out.println("") was somehow forcing an I/O that at some point was flushing cpu's registers. – Claudio May 29 '14 at 17:45
  • Totally agree with the busy spin part, though – Claudio May 29 '14 at 17:46
  • @Claudio The fact that it's in a busy-wait loop, even without the GUI, necessarily means he's expecting something outside that thread to set the flag. – chrylis -cautiouslyoptimistic- May 29 '14 at 17:49
  • From the comments, It appears to be a fault on the method of code. This is my first time coding. Would you provide an alternate solution for the main class to wait for the win condition before moving on? – DarkDestry May 29 '14 at 17:52
0

You have a busy-wait loop that constantly checks.

To alleviate you might do:

Thread,sleep(100L);

instead if the System.out.prinntln.

It is not a clean solution.

By the way: Instead if

        hasWon = hasWon && (board[i][z].getStatus() == winCond);

this is a bit faster

        if (board[i][z].getStatus() != winCond) {
            return false;
        }

Better still keep a counter of winConds.

So the best solution would be removing the loop and on changing the grid, increment/decrement a counter, and on winning exit there,

Joop Eggen
  • 107,315
  • 7
  • 83
  • 138