Full screen countdown display

By Posted on

Problem

As a part of a bigger application, I wrote code which creates a display for a countdown of a given time. It is nothing fancy, but I would still appreciate a review.

I am especially concerned about:

  1. Use of JFrame – this is the second JFrame in my app, called separately by other GUI components. I read that it is not a good idea to use multiple JFrames, however in this case I don’t know what problem it could cause (and it is easier to get full screen effect, than in case JDialog).
  2. JFrame mouseListener – is it good practice (from clean code perspective) to put ActionListener code in declaration of a component? Also, the frame in my example needs to be final or field, does it make sense to make container a final, if it is a ‘base’ component for some part of the code?
  3. Is my code readable? I know it is short, but I’m trying to get a grasp on clean code principles, naming, etc.
  4. Use of final – is it worth to declare final variables, to use in small parts of whole app, or only for global variables?
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;

public class Countdown{
    private JFrame frame;
    private Timer timer;
    private JLabel count;
    private JLabel message;
    private int[] time;
    final int MIN = 0;
    final int SEC = 1;

    public Countdown(){
        frame = new JFrame();
        frame.setUndecorated(true);
        frame.setExtendedState(Frame.MAXIMIZED_BOTH);
        frame.setAlwaysOnTop(true);
        frame.addMouseListener(new MouseAdapter() {
            @Override
            public void mouseClicked(MouseEvent e) {
                frame.setVisible(false);
                frame.dispose();
                frame.setUndecorated(!frame.isUndecorated());
                frame.revalidate();
                frame.setVisible(true);
            }
        });
        frame.add(addText());
        frame.setVisible(true);
    }
    public JPanel addText(){
        count = new JLabel();
        count.setFont(new Font("Arial Black", Font.BOLD, 200));
        count.setHorizontalAlignment(SwingConstants.CENTER);
        count.setBackground(Color.RED);
        message = new JLabel();
        message.setFont(new Font("Arial Black", Font.BOLD, 100));
        JPanel panel = new JPanel(new GridLayout(2,1));
        panel.add(count);
        panel.add(message);
        panel.setBackground(Color.WHITE);
        message.setHorizontalAlignment(SwingConstants.CENTER);
        return panel;
    }

    public void displayTime(int min, int sec){
        String minute = String.format("%02d",min);
        String second = (String.format("%02d",sec));
        count.setText(minute + ":" + second);
    }

    public void startCountdown(String time, String message){
        this.time = convertTimeToInt(time.split(":"));
        this.message.setText(message);
        count.setText(time);
        timer = new Timer(1000,new TimerListener());
        timer.setRepeats(true);
        timer.start();

    }
    public int[] convertTimeToInt(String[] time){
        int[] converted = new int[time.length];
        for(int i = 0; i<time.length; i++){
            converted[i] = Integer.valueOf(time[i]);
        }
        return converted;
    }

    private class TimerListener implements ActionListener {
        @Override
        public void actionPerformed(ActionEvent e) {
            if(time[MIN] == 0 && (time[SEC] == 1 || time[SEC] == 0)){
                count.setText("");
                message.setText("END");
                timer.stop();
            }else if(time[SEC] > 0){
                time[SEC] -= 1;
                displayTime(time[MIN], time[SEC]);
            }else if(time[SEC] == 0){
                time[SEC] = 59;
                time[MIN] -= 1;
                displayTime(time[MIN], time[SEC]);
            }

            count.setForeground(time[MIN] == 0 && time[SEC] >= 6 ? Color.BLACK : Color.RED);
        }
    }
    public static void main(String[] args){
        Countdown countdown = new Countdown();
        countdown.startCountdown("00:12","RUN");
    }
}

As a side note, and this portion is not necessarily on-topic, however I would be grateful if someone would also explain to me shortly. Why, when the main method is in same class as my countdown code, it works well, but if main is in another class, and the countdown method is called from another class (but there is no other method working), the whole app crashes and freezes, unless I call it in another thread? Is it a general rule or is my code flawed?

Solution

Let’s start with 3. I think the most important improvement would be a separation of the program logic from the layout stuff. This will also support you in creating unit tests. (In addition to that, the separate main class is working fine for me.)

I added some comments to minor changes in the code.

Main.java

public class Main
{
    public static void main( String[] args )
    {
        new CountDownFrame( new Countdown( "00:12", "RUN" ) );
    }
}

CountDownFrame.java

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

public class CountDownFrame extends JFrame // extract layout to own class
{
    private static final long   serialVersionUID    = 1L;

    private Timer                   timer;
    private JLabel                  count;
    private JLabel                  message;

    private Countdown               countdown;

    public CountDownFrame( Countdown countdown )
    {
        this.countdown = countdown;
        setUndecorated( true );
        setExtendedState( Frame.MAXIMIZED_BOTH );
        setAlwaysOnTop( true );
        addMouseListener( new MouseAdapter()
        {
            @Override
            public void mouseClicked( MouseEvent e )
            {
                setVisible( false );
                dispose();
                setUndecorated( !isUndecorated() );
                revalidate();
                setVisible( true );
            }
        } );
        add( initText() );
        startTimer();
        setVisible( true );
    }

    private JPanel initText() // rename and private
    {
        count = new JLabel();
        count.setFont( new Font( "Arial Black", Font.BOLD, 200 ) );
        count.setHorizontalAlignment( SwingConstants.CENTER );
        count.setBackground( Color.RED );
        message = new JLabel();
        message.setFont( new Font( "Arial Black", Font.BOLD, 100 ) );
        JPanel panel = new JPanel( new GridLayout( 2, 1 ) );
        panel.add( count );
        panel.add( message );
        panel.setBackground( Color.WHITE );
        message.setHorizontalAlignment( SwingConstants.CENTER );
        return panel;
    }

    private void startTimer() // rename
    {
        count.setText( countdown.toReadableString() ); // extract method
        message.setText( countdown.getMessage() );
        timer = new Timer( 1000, new TimerListener() );
        timer.setRepeats( true );
        timer.start();
    }

    private class TimerListener implements ActionListener
    {
        @Override
        public void actionPerformed( ActionEvent e )
        {
            countdown.decrement(); // separate layout from logic
            count.setForeground( countdown.isCloseToEnd() ? Color.RED : Color.BLACK ); // extract conditions
            if( countdown.isFinished() ) // extract conditions
            {
                count.setText( "" );
                message.setText( "END" );
                timer.stop();
            }
            else
            {
                count.setText( countdown.toReadableString() );
            }
        }
    }
}

The import Countdown.java with the program logic is now really straight forward and easy to read.

public class Countdown
{
    private String  message;
    private int     minutes; // no need for an array, keep it simple and readable
    private int     seconds;

    public Countdown( String time, String message )
    {
        this.message = message;
        String[] parts = time.split( ":" );
        // TODO add some error handling
        this.minutes = Integer.valueOf( parts[0] );
        this.seconds = Integer.valueOf( parts[1] );
    }

    public boolean isCloseToEnd() // move timer logic to countdown
    {
        return minutes < 1 && seconds <= 5; // opposite condition
    }

    public boolean isFinished()
    {
        return minutes < 1 && seconds < 1;
    }

    public Countdown decrement()
    {
        if( seconds > 0 ) seconds--;
        else
        {
            seconds = 59;
            minutes--;
        }
        return this; // just for chaining in test :)
    }

    public String toReadableString()
    {
        return String.format( "%02d:%02d", minutes, seconds ); // unify to one format
    }

    public String getMessage()
    {
        return message;
    }
}

And is also covered by tests without any layout interactions:

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import org.junit.Test;

public class CountdownTest
{
    @Test
    public void parser_valid()
    {
        assertEquals( "00:00", new Countdown( "0:00", null ).toReadableString() );
        assertEquals( "10:15", new Countdown( "10:15", null ).toReadableString() );
    }

    public void parser_invalid()
    {
        // TODO
        // new Countdown( "", null ));
        // new Countdown( "0", null ));
        // new Countdown( "0:0:0", null ));
        // new Countdown( "0:70", null ));
    }

    @Test
    public void isFinished()
    {
        assertTrue( new Countdown( "0:00", null ).isFinished() );
        assertFalse( new Countdown( "0:01", null ).isFinished() );
        assertFalse( new Countdown( "1:00", null ).isFinished() );
        assertFalse( new Countdown( "3:25", null ).isFinished() );
    }

    @Test
    public void isCloseToEnd()
    {
        assertTrue( new Countdown( "0:00", null ).isCloseToEnd() );
        assertTrue( new Countdown( "0:01", null ).isCloseToEnd() );
        assertTrue( new Countdown( "0:05", null ).isCloseToEnd() );
        assertFalse( new Countdown( "0:06", null ).isCloseToEnd() );
        assertFalse( new Countdown( "1:00", null ).isCloseToEnd() );
        assertFalse( new Countdown( "3:25", null ).isCloseToEnd() );
    }

    @Test
    public void decrement()
    {
        // TODO undefined
        // assertEquals( ...,  new Countdown( "0:00", null ).decrement().toReadableString() );
        assertEquals( "00:00",  new Countdown( "0:01", null ).decrement().toReadableString() );
        assertEquals( "00:04",  new Countdown( "0:05", null ).decrement().toReadableString() );
        assertEquals( "00:59",  new Countdown( "1:00", null ).decrement().toReadableString() );
        assertEquals( "03:59",  new Countdown( "4:00", null ).decrement().toReadableString() );
        assertEquals( "04:00",  new Countdown( "4:01", null ).decrement().toReadableString() );
    }

}

Please keep in mind that some corner cases where undefined in you initial program and I tried not to add any new functionality.

2.) I’m not sure if it is good practice or even best practice, but at least it is common practice.

1.) Somebody else need to answer this part.

4.) The usage of final all over the project or not is more a religions discussion. It’s the same with visibility. There are valid reasons to restrict possible changes to a minimum and there are other valid case to keep it as open as possible. Declaring variables final is in general intended to prevent changing them by accident. Short and clean methods will also address this issue and seeing final all over the place, makes the code not more readable from my point of view.

Leave a Reply

Your email address will not be published. Required fields are marked *