Why my Java alarm clock code doesn't work properly?

Question

I want the code to trigger the JOptionPane.

Here is the code for the working clock:

new Thread() {
    public void run() {
        while (true) {
            GregorianCalendar cal = new GregorianCalendar();
            int hour = cal.get(GregorianCalendar.HOUR);
            int min = cal.get(GregorianCalendar.MINUTE);
            int sec = cal.get(GregorianCalendar.SECOND);
            int AM_PM = cal.get(GregorianCalendar.AM_PM);

            String day_night;
            if (AM_PM == 1) {
                day_night = "PM";
            } else {
                day_night = "AM";
            }

            String time = hour + ":" + min + ":" + sec + " " + day_night;
            lblClock.setText(time);
        }
    }
}.start();

Here is code I wrote to trigger alarm, but no 'play sound' is coded yet, because I can't even get the JOptionPane to appear. Why? I want to get the values from spinners, than compare to real time until they meet and than trigger alarm and exit thread. How to fix it?

btnAlarm.addMouseListener(new MouseAdapter() {
    @Override
    public void mouseClicked(MouseEvent arg0) {
        new Thread() {
            public void run() {
                txtAlarmSet.setVisible(true);
                boolean flag = false;
                GregorianCalendar g = new GregorianCalendar();
                int hour = Integer.parseInt(spinnerHour.getModel().getValue().toString());
                int minute = Integer.parseInt(spinnerMinute.getModel().getValue().toString());
                int second = Integer.parseInt(spinnerSecond.getModel().getValue().toString());
                int AMorPM;
                if (rdbtnAm.isSelected()) {
                    AMorPM = 0;
                } else
                    AMorPM = 1;
                while (flag == false) {

                    int realHour = g.get(GregorianCalendar.HOUR);
                    int realMinute = g.get(GregorianCalendar.MINUTE);
                    int realSecond = g.get(GregorianCalendar.SECOND);
                    int realAM_PM = g.get(GregorianCalendar.AM_PM);
                    if (hour == realHour && minute == realMinute && second == realSecond
                            && AMorPM == realAM_PM) {
                        JOptionPane.showMessageDialog(null, "WORKS!"); // <- this doesn't appear!
                        flag = true;
                    }
                }
                txtAlarmSet.setVisible(false);
            }
        }.start();
    }
});

1. Use a [SimpleDateFormat](http://docs.oracle.com/javase/8/docs/api/java/text/SimpleDateFormat.html) to format the time. 2. Add `System.out.println` statements (or use a debugger/logger) to trace through your code. 3. `setVisible(false)` will hide the component, not show it. — Dave Jarvis, Jun 03 '16 at 19:15
Also worth noting: the `Calendar.HOUR` field ranges from 0-11. If your alarm is set for 12:01PM, the value you get back will be 0:01. — Ian McLaird, Jun 03 '16 at 19:18
@cricket_007 won't the `while (flag == false)` line keep the thread going? I don't see any reason why the thread should ever terminate, since `g` value is never updated. — Ian McLaird, Jun 03 '16 at 19:27
Please accept an answer by clicking the green checkmark next to it to indicate when a problem is solved and what was effective. — Brandon McKenzie, Jun 03 '16 at 19:48
Please do not edit your post to fix the problem. That's what the answers are for. — Ian McLaird, Jun 03 '16 at 20:04
You're violating the single threaded nature of Swing, you should never be updating the UI from outside the context of the EDT, see [Concurrency in Swing](https://docs.oracle.com/javase/tutorial/uiswing/concurrency/) for more details. A simpler solution might be to calculate the difference between when you set the alarm and the time the alarm needs to be triggered and simply set up some kind of timer which is delayed for that long — MadProgrammer, Jun 03 '16 at 22:02

score 1 · Accepted Answer · answered Jun 03 '16 at 19:26

In your checking loop, you need to reacquire the Calendar on every pass, otherwise, you'll just end up re-checking the same time value over and over. Move the line

GregorianCalendar g = new GregorianCalendar();

inside the loop.

Note: This is not a particularly good approach to this problem. What you're doing is called "busy waiting" and it's generally not good for much other than making the CPU get hot. A better approach would be to use an event-driven approach, but that's beyond the scope of this answer.

score -1 · Answer 2 · answered Jun 03 '16 at 19:17

-1

One major problem I notice is missing } after AMorPM = 1; making it impossible to work for AM.

answered Jun 03 '16 at 19:17

Mr Redstoner

478
3
6
19

2

There's no opening `{` after the `else` token, though, so this is syntactically fine, even though it's a little weird. – Ian McLaird Jun 03 '16 at 19:20
Single statement if-elses aren't a major problem, only syntactically bad – OneCricketeer Jun 03 '16 at 19:31

Why my Java alarm clock code doesn't work properly?

2 Answers2