I have created a timer with GUI in Java.
Here's the code:
import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.Toolkit;
import java.awt.event.ActionListener;
import java.text.NumberFormat;
import javax.swing.text.NumberFormatter;
import static javax.swing.WindowConstants.EXIT_ON_CLOSE;
public class Counter {
private JButton button;
private JFormattedTextField hours;
private JFormattedTextField minutes;
private JFormattedTextField seconds;
private Timer timer;
private int delay = 1000;
private ActionListener taskPerformer = new ActionListener() {
public void actionPerformed(ActionEvent evt) {
if(sec >= 1) {
sec = sec - 1;
seconds.setText(String.valueOf(sec));
}
else if(sec == 0 && min > 0) {
sec = 59;
min = min - 1;
seconds.setText(String.valueOf(sec));
minutes.setText(String.valueOf(min));
}
else if(min == 0 && hrs > 0) {
sec = 59;
min = 59;
hrs = hrs - 1;
seconds.setText(String.valueOf(sec));
minutes.setText(String.valueOf(min));
hours.setText(String.valueOf(hrs));
}
if(hrs == 0 && min == 0 && sec == 0) {
Toolkit.getDefaultToolkit().beep();
JOptionPane.showMessageDialog(null,"Countdown ended!","Ende", JOptionPane.CANCEL_OPTION);
timer.stop();
}
}
};
private int hrs;
private int min;
private int sec;
public static void main(String[] args) throws InterruptedException {
SwingUtilities.invokeLater(Counter::new);
}
Counter() {
JFrame frame = new JFrame();
frame.setSize(300, 200);
frame.setDefaultCloseOperation(EXIT_ON_CLOSE);
JPanel panel = new JPanel(new BorderLayout());
JPanel subpanel1 = new JPanel(new GridLayout(2, 3));
/*
* The following lines ensure that the user can
* only enter numbers.
*/
NumberFormat format = NumberFormat.getInstance();
NumberFormatter formatter = new NumberFormatter(format);
formatter.setValueClass(Integer.class);
formatter.setMinimum(0);
formatter.setMaximum(Integer.MAX_VALUE);
formatter.setAllowsInvalid(false);
formatter.setCommitsOnValidEdit(true);
//"labeling"
JTextField text1 = new JTextField();
text1.setText("hours:");
text1.setEditable(false);
JTextField text2 = new JTextField();
text2.setText("minutes:");
text2.setEditable(false);
JTextField text3 = new JTextField();
text3.setText("seconds:");
text3.setEditable(false);
//fields for minutes and seconds
hours = new JFormattedTextField(formatter);
minutes = new JFormattedTextField(formatter);
seconds = new JFormattedTextField(formatter);
hours.setText("0");
minutes.setText("0");
seconds.setText("0");
JPanel subpanel2 = new JPanel();
/*
* When the user presses the OK-button, the program will
* start to count down.
*/
button = new JButton("OK");
button.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent actionEvent) {
hrs = Integer.valueOf(hours.getText());
min = Integer.valueOf(minutes.getText());
sec = Integer.valueOf(seconds.getText());
button.setEnabled(false);
//Timer for one second delay
Timer timer = new Timer(delay, taskPerformer);
timer.start();
}
});
//Reset-button
JButton button2 = new JButton("Reset");
button2.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent actionEvent) {
hours.setText("0");
minutes.setText("0");
seconds.setText("0");
button.setEnabled(true);
hrs = 0;
min = 0;
sec = 0;
}
});
subpanel1.add(text1);
subpanel1.add(text2);
subpanel1.add(text3);
subpanel1.add(hours);
subpanel1.add(minutes);
subpanel1.add(seconds);
subpanel2.add(button);
subpanel2.add(button2);
panel.add(subpanel1, BorderLayout.CENTER);
panel.add(subpanel2, BorderLayout.PAGE_END);
frame.add(panel);
frame.setVisible(true);
}
}
I would appreciate any suggestions on improving the code!
1 Answer 1
A few things I noticed:
Quite a few of your variables can be marked final
. From my understanding pretty much any variable where the underlying object doesn't get reassigned to a new object.
Instead of 3 separate variable for the time parts, you could use java.time.LocalTime
to hold the time parts. Not only does this reduce your code, but it simplifies the count down portion in that subtracting 1 second will adjust the other fields automatically.
Since Integer.valueOf
uses Integer.parseInt
anyway, I think it would be better, in this case, to use Integer.parseInt
You misspelled the title of the message dialog when the countdown is done.
I think INFORMATION_MESSAGE
is a better icon, for the message dialog, than CANCEL_OPTION
, in this case.
The throws
clause in the main
declaration is redundant.
With these adjustments your code could look like this:
import javax.swing.*;
import javax.swing.text.NumberFormatter;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.text.NumberFormat;
import java.time.LocalTime;
import static javax.swing.WindowConstants.EXIT_ON_CLOSE;
public class Counter {
private final JButton button;
private final JFormattedTextField hours;
private final JFormattedTextField minutes;
private final JFormattedTextField seconds;
private final Timer timer;
private final int delay = 1000;
private final ActionListener taskPerformer = new ActionListener() {
public void actionPerformed(ActionEvent evt) {
time = time.minusSeconds(1);
if (time.equals(LocalTime.MIN)) {
Toolkit.getDefaultToolkit().beep();
JOptionPane.showMessageDialog(null, "Countdown ended!", "Ended", JOptionPane.INFORMATION_MESSAGE);
timer.stop();
}
seconds.setText(String.valueOf(time.getSecond()));
minutes.setText(String.valueOf(time.getMinute()));
hours.setText(String.valueOf(time.getHour()));
}
};
private LocalTime time = LocalTime.of(0, 0, 0);
public static void main(String[] args) {
SwingUtilities.invokeLater(Counter::new);
}
Counter() {
timer = new Timer(delay,taskPerformer);
JFrame frame = new JFrame();
frame.setSize(300, 200);
frame.setDefaultCloseOperation(EXIT_ON_CLOSE);
JPanel panel = new JPanel(new BorderLayout());
JPanel subPanel1 = new JPanel(new GridLayout(2, 3));
/*
* The following lines ensure that the user can
* only enter numbers.
*/
NumberFormat format = NumberFormat.getInstance();
NumberFormatter formatter = new NumberFormatter(format);
formatter.setValueClass(Integer.class);
formatter.setMinimum(0);
formatter.setMaximum(Integer.MAX_VALUE);
formatter.setAllowsInvalid(false);
formatter.setCommitsOnValidEdit(true);
//"labeling"
JTextField text1 = new JTextField();
text1.setText("hours:");
text1.setEditable(false);
JTextField text2 = new JTextField();
text2.setText("minutes:");
text2.setEditable(false);
JTextField text3 = new JTextField();
text3.setText("seconds:");
text3.setEditable(false);
//fields for minutes and seconds
hours = new JFormattedTextField(formatter);
minutes = new JFormattedTextField(formatter);
seconds = new JFormattedTextField(formatter);
hours.setText("0");
minutes.setText("0");
seconds.setText("0");
JPanel subPanel2 = new JPanel();
/*
* When the user presses the OK-button, the program will
* start to count down.
*/
button = new JButton("OK");
button.addActionListener(actionEvent -> {
time = LocalTime.of(Integer.parseInt(hours.getText()), Integer.parseInt(minutes.getText()), Integer.parseInt(seconds.getText()));
button.setEnabled(false);
//Timer for one second delay
timer.start();
});
//Reset-button
JButton button2 = new JButton("Reset");
button2.addActionListener(actionEvent -> {
hours.setText("0");
minutes.setText("0");
seconds.setText("0");
button.setEnabled(true);
time = LocalTime.of(0, 0, 0);
timer.stop();
});
subPanel1.add(text1);
subPanel1.add(text2);
subPanel1.add(text3);
subPanel1.add(hours);
subPanel1.add(minutes);
subPanel1.add(seconds);
subPanel2.add(button);
subPanel2.add(button2);
panel.add(subPanel1, BorderLayout.CENTER);
panel.add(subPanel2, BorderLayout.PAGE_END);
frame.add(panel);
frame.setVisible(true);
}
}
-
\$\begingroup\$ would you please rename the magic number
delay = 1000
? \$\endgroup\$Martin Frank– Martin Frank2020年03月13日 06:43:15 +00:00Commented Mar 13, 2020 at 6:43
javax.swing.*
I can't tell what sort of Timer you have. There's java.util.Timer and java.swing.Timer, and it isn't obvious which one you're using. Please don't use global imports like this. \$\endgroup\$java.util
andjavax.swing
is pretty easy to see, especially when only one of them is being imported. Also, as in this case, it seems to me that using a global is better than adding a lot of extra lines to the imports \$\endgroup\$