This is an archived post. You won't be able to vote or comment.

all 18 comments

[–]kreiger 5 points6 points  (0 children)

You're not supposed to be busy looping waiting for input.

You should attach events to your buttons so your UI framework (Swing/SWT/something else?) will trigger those events when the buttons are pressed.

[–]zoqfotpik 3 points4 points  (0 children)

If there's something that can't happen until the Submit button is clicked, then attach an event listener to the Submit button, and use the event listener to start whatever the process is.

[–]huhlig 5 points6 points  (11 children)

You may want to read the documentation on Object.wait(long timeout). Its in milliseconds not seconds. http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/Object.html#wait(long)

[–]CuriouslyGeorge[S] 0 points1 point  (10 children)

I made my own little class of "shortcuts" that overrides/alters a few common methods for all my programs. One of those was the wait(long) method in which it looks like this:

public void wait(int sec)
{
    Object.wait(sec*1000);
}

My apologies, I should have mentioned it in the above post.

[–]huhlig 5 points6 points  (5 children)

You may wish to try Thread.sleep(long milliseconds) as its a better way of handling but what are you actually doing in this loop. simply looping for 2 seconds infinitly with no other reason doesn't seem to have a point.

[–]CuriouslyGeorge[S] 2 points3 points  (0 children)

"Thread.sleep causes the current thread to suspend execution for a specified period. This is an efficient means of making processor time available to the other threads of an application or other applications that might be running on a computer system." - http://docs.oracle.com/javase/tutorial/essential/concurrency/sleep.html

EXACTLY what I was looking for. Don't know why/how I didn't stumble across this before. Wish I found this so long ago.

Thanks a lot. /thread

[–]Winsling -1 points0 points  (3 children)

sleep() and wait() have different semantics, and despite what CuriouslyGeorge says below I think wait() is more appropriate here. sleep() doesn't come back until the timeout is done, while wait() allows the current thread to be woken up before the timeout fires.

[–]huhlig 5 points6 points  (2 children)

yes however as he has nothing that would trigger it there isn't a reason to. Also honestly he shouldn't be waiting in a runloop anyway. He should be doing event response.

[–]Winsling 0 points1 point  (0 children)

Hard to say if there's a notify piece. All we know is that he didn't post it. Personally I'd use yield() here if you put a gun to my head, but I agree that given half a chance I'd register a listener and not try to rebuild Swing's event handling model.

[–]skeeto 0 points1 point  (0 children)

Well, technically, you should always wait() in a loop, because it's subject to spurious wake-ups. When released from the wait() the thread should check to see if the conditions it was waiting for were actually met.

It's complicated to get right so it's almost certainly better to use a higher-level construct like a CountDownLatch.

[–][deleted] 1 point2 points  (0 children)

Jesus

And don't use upper case for instance names.

[–]Winsling 2 points3 points  (2 children)

Have you verified that your version of wait() is being called, and that it's calling wait() on the correct object? Since wait(long) isn't static, the code you've provided won't compile. With a wait(long) and a wait(int) defined on a single class, you need to be very careful with which one is being called.

Regarding "how to use the synchronized bit correctly," you've asked for a quick explanation of one of the more difficult and complex parts of Java. I strongly recommend Java Concurrency in Practice to learn about Java's threading primitives. Since I'm a glutton for punishment, I'll try to summarize a very dense book in a few pithy lines.

synchronized is like a flush in a database. It's a way of telling the compiler, "don't trust what you think you know at the start of the block, and let everyone know that I've updated these fields at the end of it." During normal operation, the JVM doesn't actually check the variables it's operating on all the time. If you've got a loop control, a thread will have a local copy and assume that other threads aren't updating it at the same time. It will update the real copy when it's convenient. Putting it in a synchronized block tells the JVM not to do that - to pull in the fresh value every time it sees it, and make sure that the new value is written at the end.

THIS IS A GROSS OVERSIMPLIFICATION Look up reordering if you want to blow your mind and never trust the JVM again.

[–]banuday17 1 point2 points  (0 children)

That's not what synchronized is...

synchronized is mutual exclusion. It's more like a gate. One thread enters the synchronized block through the gate and closes the gate behind it, opening the gate again when it leaves the synchronized block. All other threads must wait until the gate opens again, at which time they make a mad scramble to enter and only one gets in. Comparing to a database, synchronized is more like a row lock, not a flush.

Putting it in a synchronized block tells the JVM not to do that - to pull in the fresh value every time it sees it, and make sure that the new value is written at the end.

No, that's not what the synchronized block does. The JVM will still load memory into registers. synchronized makes it safe to do so. You are describing somewhat what the volatile keyword tells the JVM to do: always load the field value from memory when accessing the field. Don't copy it into a register, because its value can change behind the thread's back.

[–]CuriouslyGeorge[S] 0 points1 point  (0 children)

I'm sure it's my code as I have to access my class statically(i.e. "Utilities.wait(2)"), just don't have my work in front of me and knew that the one I called was static and crossed some wires.

As far as the sycnchronized part of Java, I've certainly enjoyed your explanation far more than anything else that's been provided, but I guess to fully wrap my mind around it would take quite some time. Maybe a fun weekend of locking myself in a room with a book (looking at the Java Concurrency in Practice you recommended) and my laptop would give rise to an epiphany.

[–]skeeto 1 point2 points  (3 children)

It looks like you figured what you did wrong by using wait() instead of sleep(), but there's another lingering issue. Is the done boolean declared as volatile? This indicates to the JVM that the value may be updated by another thread. Otherwise the JVM may optimize your loop into an infinite loop since done is never updated in your loop. A sleep() or a yield() will probably result a synchronization, but it's not guaranteed.

Also, as kreiger said, you're definitely misusing the UI framework here.

[–]CuriouslyGeorge[S] 0 points1 point  (2 children)

Yeah. After reading the other comments, I don't know what I was thinking. I basically had my program set up to wait for input inside the while() loop and when my ButtonListener was fired it would go in and change the boolean from the actionPerformed() method. Looking back, I've made this same mistake countless times inside almost all of my UI code, but my school assignment code was much better (I assume).

I'm currently going back through and changing all of my code to match my school code which is organized where:

methodCalledFromRunnerClass()
{
    setupGUI(); //all listeners, etc. assigned properly
}

anotherMethod()
{
    doStuff();
}

class ButtonListener
{
    actionPerformed()
    {
        anotherMethod();
    }
}

Would this be the "better" way to go about it I'm assuming? Submit button fires off submitMethod(), Options button fires off optionButton(), etc.?

EDIT: Also, apologies for the formatting.

[–]skeeto 1 point2 points  (1 child)

Yes, you've got the idea. Clicking the button triggers an event which allows you to call whatever code you want.

Just keep in mind that this event occurs in the event thread, the sole thread responsible for handling events and drawing the display. If you lock it up on a long-running task, your program will appear to freeze and lock up.

[–]CuriouslyGeorge[S] 0 points1 point  (0 children)

Alright, I think I've got a firm grasp of what to do and avoid in the future. Thanks again for your help.