HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Java GUI code with Swing Timer

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withjavaswingtimercodegui

Problem

I have the following questions for the below code:

  • Is there a simpler implementation of an on-screen counter?



  • I made CountTimer inner class since it's tightly coupled with the GUI part anyway. What would be the best way to uncouple them?



```
package count_timer;

import java.awt.*;
import java.awt.event.*;
import javax.swing.*;

public class CountTimerGUI implements ActionListener {

private JFrame frame;
private JPanel panel;

private JLabel timeLabel = new JLabel();

private JButton startBtn = new JButton("Start");
private JButton pauseBtn = new JButton("Pause");
private JButton resumeBtn = new JButton("Resume");
private JButton stopBtn = new JButton("Stop");
private JButton resetBtn = new JButton("Reset");

private JButton greenBtn = new JButton("Green");
private JButton redBtn = new JButton("Red");

private CountTimer cntd;

public CountTimerGUI() {
setTimerText(" ");
GUI();
}

private void GUI() {
frame = new JFrame();
panel = new JPanel();

panel.setLayout(new BorderLayout());
timeLabel.setBorder(BorderFactory.createRaisedBevelBorder());
panel.add(timeLabel, BorderLayout.NORTH);

startBtn.addActionListener(this);
pauseBtn.addActionListener(this);
resumeBtn.addActionListener(this);
stopBtn.addActionListener(this);
resetBtn.addActionListener(this);
greenBtn.addActionListener(this);
redBtn.addActionListener(this);

JPanel cmdPanel = new JPanel();
cmdPanel.setLayout(new GridLayout());

cmdPanel.add(startBtn);
cmdPanel.add(pauseBtn);
cmdPanel.add(resumeBtn);
cmdPanel.add(stopBtn);
cmdPanel.add(resetBtn);

panel.add(cmdPanel, BorderLayout.SOUTH);

JPanel clrPanel = new JPanel();
clrPanel.setLayout(new GridLayout(0,1));

clrPanel.add(greenBtn);
clrPanel.add(redBtn);

panel.add(clrPan

Solution

GUI:

My preference is to have one ActionListener per button. This way you won't have to deal with checking the source before performing an action.

Naming the method GUI() doesn't follow naming conventions. Methods should start with a lower case letter. initGUI() would be a better name as it also describes what is happening in the method.

The CountTimer is not a GUI component, it should not be in the GUI initialization method.

CountTimer:

AS you noted, CountTimer is tightly coupled with your GUI. This means you will have to do work in order to reuse it in any way. It would be better as a stand-alone class that you just happen to use in a GUI.

I believe the following would be a good interface. It provides that same kind of control and allows the user of the counter to decide what to do when an event occurs.

class `CountTimer` {
  CountTimer(TickListener listener);
  void start();
  void stop();
  void pause();
  void resume();
  void reset();
  interface TickListener {
    tick(int count);
  }
}


You don't set isTimerActive to false when you stop the timer. In general, this field does not give you anything because stopping the timer will stop events from being fired.

TimeFormat:

Again, this function does not follow naming conventions.

This should be code that is coupled with the GUI as it deals with the presentation of a number you are tracking, not the actual number value.

You can pass multiple arguments to String.format(), so you don't need to call it three times. Just create one format sting that accepts all the values you want.

General:

Star imports can pollute your namespace and make it hard to track down where a class is coming from.

Code Snippets

class `CountTimer` {
  CountTimer(TickListener listener);
  void start();
  void stop();
  void pause();
  void resume();
  void reset();
  interface TickListener {
    tick(int count);
  }
}

Context

StackExchange Code Review Q#40136, answer score: 5

Revisions (0)

No revisions yet.