0

I wrote a complex java program, that has a GUI.

In the GUI I have three buttons, "play", "play all" and "reset", and some table.

When I click play, the program does its logic and then prints the output to the table i mentioned. this is working fine, as intended. after a certain amount of "play" operations, we printed all the data to the table and so we are done and can no longer click it. at the point, i disable the play button.

When I click play all, I want it to be as if I clicked "play" until it is no longer possible. So I wrote this code:

public synchronized void actionPerformed(ActionEvent event) 
{
    if(event.getSource() == playAllButton)
    {
        while(playButton.isEnabled())
        {
            playButton.doClick();
            try {
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
        }
    }
    if(event.getSource() == playButton)
    {
        Command command = listOfCommands.get(commandNumber++);
        if(command.getCommandType() == "PF")
        {
            PFLabel.setText("PF Count:" + (++PFCount));
        }
        if(command.getCommandType() == "PR")
        {
            PRLabel.setText("PR Count:" + (++PRCount));
        }
        if(command.getCommandType() == "GP")
        {
            if(checkIfPF())
            {
                addPageToRam(((GPCommand)command).getPage());
            }
            else
            {
                if(checkIfPR(((GPCommand)command).getPage().getPageId()))
                {
                    replacePage(((GPCommand)command).getPage(),getWithWhoToReplace(((GPCommand)command).getPage().getPageId()));
                }
            }
        }
    }
    if(commandNumber == listOfCommands.size()) // we are done!
    {
        playButton.setEnabled(false);
    }
}

As I mentioned, when I click the play button, it immediately prints the result as intended. But when I click play all, it runs for a very long time without displaying any output, and then at the end prints just the final result. This isn't what I want. What I want is for it to print the current result, wait a bit so I can look at it, and then print the next result.

Even when I remove the Thread.sleep it is very slow (and still only prints the last result).

Is there a way to make it run faster? And more importantly - how do I make it wait before clicking the button again? I want to view the current result for a while before it clicks again.

Oria Gruber
  • 1,513
  • 2
  • 22
  • 44
  • Is it normal that you are comparing String with `==`? `command.getCommandType() == "PF"` – Tunaki May 20 '16 at 10:40
  • Yes, that's normal and working as intended. – Oria Gruber May 20 '16 at 10:41
  • Fine, even if I fix it (which I will and thanks for that), how does it help with the current issue? – Oria Gruber May 20 '16 at 10:42
  • I thoughts maybe instead of playButton.doClick I can call ActionPerformed (recursively) with the playButton action, but I don't know how to do that without doing a playButton.click – Oria Gruber May 20 '16 at 10:47
  • @OriaGruber Yes, you are right. I kinda assumed, that you had an else if. Besides, I was wondering if you couldn't use a for-loop, and pack the functionality into methods? Could you put the part of the playButton into a method, and reuse it in a for-loop in the playAllButton-part? Since you have already your maximum amount of iterations in listOfCommands.size(), you could also eliminate the commandNumber-part, because you have the iterator-variable in the for-loop. – KJaeg May 20 '16 at 11:04
  • @OriaGruber Another thought: How do you define "slow"? Does it look like the GUI hangs or freezes? Or do you have an exact feedback about the performance (time)? – KJaeg May 20 '16 at 11:08
  • Where do you repaint your GUI? Maybe it does just repaint after `actionPerformed` finished? That would explain why you can only see the last result when cliking the _play all_ button. – Tobias Brösamle May 20 '16 at 11:09
  • By slow I mean the GUI freezes and becomes unresponsive (cant even close it) for about a minute, and then it only prints the final result. printing is performed in two places: the PF.setText and PR.setText, and also at replacePage. and also at addPageToRAM – Oria Gruber May 20 '16 at 11:12
  • @OriaGruber I think, that Tobias is on the right track. If the GUI freezes, the code is doing something in the background. For long tasks, especially with a waiting time, I write them as asynchronous tasks (SwingWorkers), so the GUI stays usable while the job is performing in the background. That way you can manipulate e.g. labels during the code exeuction, without waiting for the code to be finished. Otherwise, if the code execution is done, you will only see the last result, after the GUI is responsive again. – KJaeg May 20 '16 at 11:20
  • At those places you call `repaint` method? So `PFLabel` and `PRLabel` are not just `JLabel`s? Sorry, but in that case I have no idea what's going wrong. – Tobias Brösamle May 20 '16 at 11:22
  • Ah, sorry, I thought you meant to ask where I actually change whats being displayed in the GUI. Nowhere in my code am I using the repaint method. All I'm doing is changing labels and contents of tables. – Oria Gruber May 20 '16 at 11:24
  • the code doesn't do anything complicated in the background. when i click the play button once, it immediately displays the result. – Oria Gruber May 20 '16 at 11:29
  • Then try to add a `repaint` directly after `playButton.doClick()`, meaning try to rrepaint the gui. I'm not 100% sure, but I think it will at least show every result and not only the last one. – Tobias Brösamle May 20 '16 at 11:31
  • I think, you still should try to use it asynchronous. See here: http://stackoverflow.com/questions/782265/how-do-i-use-swingworker-in-java/782309#782309 – KJaeg May 20 '16 at 11:34
  • also im not sure if its related, but when i click the playall button, the play button also looks is if its being held, its not being released. its as if i clicked it with a mouse and im holding it. repaint didn't change anything. this is what i meant by ignoring thread.sleep. i want it to click and wait. instead of just clicks endlessly without waiting at all. – Oria Gruber May 20 '16 at 11:35
  • @OriaGruber Yes, this is common if you have a longer task behind the button. Before the button is released, the code is finished. Which also means, that only your last result in the label will be shown. If you call the code in an asynchronous task, the button will be released, and the GUI will not look like it is freezed. Additionally the asynchronous task will set the label as soon as the setText() is called. – KJaeg May 20 '16 at 11:37
  • But that's why I'm using thread.sleep. to give it time to finish the logic of play button click and print the result. – Oria Gruber May 20 '16 at 11:40
  • @OriaGruber No, it will not give the code time to finish. Instead it will also interpret your sleeping time as performance duration. This means, that setting a sleeping time of 1 second, will look like the GUI is freezing 1 second longer. It works that way, because the waiting time is still part of the execution behind the click on the playAllButton. – KJaeg May 20 '16 at 11:42
  • @OriaGruber Please check the link I posted above. The example there is quite close to your code piece. – KJaeg May 20 '16 at 11:44
  • 1
    Yeah.. if you call Sleep(x) in a GUI app display thread, NOTHING happens for the inerval 'x'. No repaints, no nothing. Sleep() removes all execution from the calling thread. – Martin James May 20 '16 at 11:58

1 Answers1

1

In the comments section I figured out, that the issue could be solved by using SwingWorkers and moving the long-time-performing code to an asynchronous task. Mr. Coobird has posted a way, how to do this here.

Community
  • 1
  • 1
KJaeg
  • 698
  • 3
  • 7
  • 23
  • 1
    Yes. You should never use Thread.sleep to give the illusion of a limited-speed looping thread. If you do so, it will simply stop the entire process until the sleep time completes. – serebit May 20 '16 at 12:09
  • Just tried this using a simpler programm (it's just counting up to 10 using `doClick()` method of a button) using SwingWorker - it works perfectly fine. I could even watch the numbers counting (pretty fast, of course) without adding any way to make it run slower. But it's also possible to send the worker to sleep for some time so you can verify the data while it counts. So thanks for posting the link, didn't know about SwingWorker before. – Tobias Brösamle May 20 '16 at 12:56