Simple Java Game including Thread

Question

I am a Java beginner and have made a simple Compare Game. It has done well but I am not sure about my solution, especially about my thread. Can you give some advice on making it better?

public class CompareGame {
 static short first=0;
 static short second=0;
 static JLabel lblFirst = new JLabel("First");
 static JLabel lblSecond = new JLabel("Second");
 private JFrame frame;
 static String math = null;
 JButton btnReset = new JButton("Reset");
 JButton btnGo = new JButton("Go");
 public static void main(String[] args) {
 EventQueue.invokeLater(new Runnable() {
 public void run() {
 try {
 CompareGame window = new CompareGame();
 window.frame.setVisible(true);
 } catch (Exception e) {
 e.printStackTrace();
 }
 }
 });
 }

Main frame:

public CompareGame() {
 initialize();
 }
 private void initialize() {
 frame = new JFrame("CompareGame");
 frame.setBounds(100, 100, 432, 279);
 frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 frame.getContentPane().setLayout(null);
 JLabel lblWelcome = new JLabel("Welcome");
 lblWelcome.setHorizontalAlignment(SwingConstants.CENTER);
 lblWelcome.setBounds(161, 30, 81, 14);
 frame.getContentPane().add(lblWelcome);
 lblFirst.setBounds(88, 75, 46, 14);
 frame.getContentPane().add(lblFirst);
 JLabel lblMath = new JLabel("Math");
 lblMath.setHorizontalAlignment(SwingConstants.CENTER);
 lblMath.setBounds(178, 75, 46, 14);
 frame.getContentPane().add(lblMath);
 lblSecond.setBounds(285, 75, 46, 14);
 frame.getContentPane().add(lblSecond);
 JButton btnLeft = new JButton("<");
 btnLeft.addActionListener(new ActionListener() {
 public void actionPerformed(ActionEvent e) {lblMath.setText("<");
 }
 });
 btnLeft.setBounds(38, 114, 89, 23);
 frame.getContentPane().add(btnLeft);
 JButton btnMid = new JButton("=");
 btnMid.addActionListener(new ActionListener() {
 public void actionPerformed(ActionEvent e) {lblMath.setText("=");
 }
 });
 btnMid.setBounds(166, 114, 89, 23);
 frame.getContentPane().add(btnMid);
 JButton btnRight = new JButton(">");
 btnRight.addActionListener(new ActionListener() {
 public void actionPerformed(ActionEvent e) {lblMath.setText(">");
 }
 });
 btnRight.setBounds(285, 114, 89, 23);
 frame.getContentPane().add(btnRight);
 class Countdown implements Runnable{
 public void run(){
 long time=System.currentTimeMillis(); 
 long future=time+2*1000; //2 seconds countdown time
 while(time<future){time=System.currentTimeMillis();}
 btnGo.doClick();
 }
 }
 btnReset.addActionListener(new ActionListener() {
 public void actionPerformed(ActionEvent arg0) {
 lblWelcome.setText(null);
 lblMath.setText("?");
 btnLeft.setEnabled(true);btnMid.setEnabled(true);
 btnRight.setEnabled(true);
 math();
 Countdown Time=new Countdown();
 new Thread(Time).start();
 }
 });
 btnReset.setBounds(166, 192, 89, 23);
 frame.getContentPane().add(btnReset);
 btnGo.addActionListener(new ActionListener() { // I used button to do job
 public void actionPerformed(ActionEvent e) {
 String result=lblMath.getText();
 if(result==math)
 {lblMath.setText("?");
 math();
 Countdown Time=new Countdown();
 new Thread(Time).start();}
 else
 {lblWelcome.setText("Game Over");
 btnLeft.setEnabled(false);btnMid.setEnabled(false);
 btnRight.setEnabled(false);}
 }
 });
 btnGo.setBounds(1, 1, 1, 1);
 frame.getContentPane().add(btnGo);
 btnGo.setVisible(false);
 }
 private static String math(){ // creat number
 Random random=new Random();
 first=randomShort(1, 100, random );
 lblFirst.setText(String.valueOf(first));
 second=randomShort(1,100,random);
 lblSecond.setText(String.valueOf(second));
 if (first<second) math="<";
 if (first==second)math="=";
 if (first>second) math=">";
 return math;
 }
 private static short randomShort(int i, int j, Random random) { //random number
 short range = (short) (j - i + 1);
 short fraction = (short) (range * random.nextDouble());
 short randomNumber = (short) (fraction + i);
 return randomNumber;} 
}

Answer 1

1. Your code formatting looks broken(indentation is inconsistent, opening/closing curly brackets are located in strange places, multiple statements in one line, etc). It makes your code very hard to read.

2. Operators should be surrounded by whitspaces. For instance,

   long future = time + 2 * 1000;
   

   is more readable than

   long future=time+2*1000;
   

3. Variables and methods names are not self-descriptive in your code. For example, a name math() for a method is bad: it does not reflect what this method actually does. You could call it generateNumbers() or something like that. The same goes for variables: for instance, math is pretty a strange name for a variable(and again, it does show the meaning of this variable). Moreover, there is no point in mangling words: buttonLeft is a better name than btnLeft, labelFirst or firstLabel is more comprehensive than lblFirst and so on.

4. Creating your own class that works like a timer(Countdown) is a bad idea. A timer is already present in Java standard library: java.swing.Timer, so you can use it.

5. You can make you code more concise by using lambda-expressions if you have Java8. For example, you can add a listener to a button like this:

   button.addActionListener((ActionEvent e) -> {
    // do something
   });
   

• Shade: I would consider long future = time + 2*1000; more readable then either example you have. It nudges your brain to evaluate with correct precedence. Not all places that allow whitespace deserve it.
• kraskevich: @Shade To me, any binary operator not wrapped around with whitespaces looks worse than with them. Moreover, according to Oracle Java coding conventions(and other style guides like Google code style) whitespaces should be there.

Answer 2

You use a busy loop for a timeout. There is a better method with javax.swing.Timer. This removes the need for btnGo and the thread.

private Timer timer;// as field
timer = new Timer(2*1000, new ActionListener(){
 public void actionPerformed(ActionEvent e) {
 String result=lblMath.getText();
 if(result==math) {
 lblMath.setText("?");
 math();
 }else{
 lblWelcome.setText("Game Over");
 btnLeft.setEnabled(false);
 btnMid.setEnabled(false);
 btnRight.setEnabled(false);
 timer.stop();
 }
 }
});
timer.setRepeats(true);//auto restarts the timer after it triggers

Then when you need to start it you just call timer.start();.

Other than that please keep a consistent format, especially the indentation and brace style. It will vastly increase readability. Compare the listener I provided to your btnGo listener.

Also the fixed layout of the components will do odd things when the user tries to resize the window, I suggest using a layout manager.

• Nongdan Xitrum: Thank you. I'll learn javax.swing.Timer. and add it to my app.
• Nongdan Xitrum: timer.stop() can not put in that field, I can not fix it. Please help me.