3
\$\begingroup\$

I was in the mood for doing some GUI coding, and eventually I came with this simple program: you enter a date, press the button, and the program tells you temporal difference in days between the input date and current date.

package net.coderodde.gui.date;

import java.awt.Dimension;
import java.awt.GridLayout;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Calendar;
import java.util.Date;
import javax.swing.JButton;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JSlider;
import javax.swing.JTextField;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;

public class DaysCounter implements ChangeListener, ActionListener {

    /**
     * The actual frame of this program.
     */
    private final JFrame frame;

    /**
     * The label displaying the currently selected day of month.
     */
    private final JLabel dayLabel;

    /**
     * The slider for choosing the day of month.
     */
    private final JSlider daySlider;

    /**
     * The combo box for choosing the month.
     */
    private final JComboBox monthComboBox;

    /**
     * The text field for inputting the year.
     */
    private final JTextField yearTextField;

    /**
     * The button for initiating the age calculation.
     */
    private final JButton computeButton;

    /**
     * Constructs the graphical user interface.
     */
    public DaysCounter() {
        // Construct components:
        this.frame = new JFrame("Age in days");
        this.dayLabel = new JLabel("16");
        this.daySlider = new JSlider(1, 31, 16);

        String[] months = new String[] {
            "January",
            "February",
            "March",
            "April",
            "May",
            "June",
            "Jule",
            "August",
            "September",
            "October",
            "November",
            "December",
        };

        this.monthComboBox = new JComboBox(months);
        this.yearTextField = new JTextField("1980");
        this.computeButton = new JButton("Compute");

        // Add listeners:
        daySlider.addChangeListener(this);
        computeButton.addActionListener(this);

        // Align the selected day field horizontally.
        dayLabel.setHorizontalAlignment(JLabel.CENTER);

        // Add the components to the frame.
        frame.getContentPane().setLayout(new GridLayout(5, 1));
        frame.getContentPane().add(dayLabel);
        frame.getContentPane().add(daySlider);
        frame.getContentPane().add(monthComboBox);
        frame.getContentPane().add(yearTextField);
        frame.getContentPane().add(computeButton);

        frame.pack();

        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setResizable(false);

        // Center out the frame.
        Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();

        frame.setLocation((dim.width - frame.getWidth()) / 2,
                          (dim.height - frame.getHeight()) / 2);

        frame.setVisible(true);
    }

    public static void main(final String... args) {
        javax.swing.SwingUtilities.invokeLater(() -> {
            new DaysCounter();
        });
    }

    /**
     * Change the current day of month.
     * 
     * @param e the event object.
     */
    @Override
    public void stateChanged(ChangeEvent e) {
        dayLabel.setText("" + ((JSlider) daySlider).getValue());
    }

    /**
     * Initiates the age calculation.
     * 
     * @param e the button press event, ignored.
     */
    @Override
    public void actionPerformed(ActionEvent e) {
        Calendar start = Calendar.getInstance();
        Calendar end = Calendar.getInstance();

        int year;

        try {
            year = Integer.parseInt(yearTextField.getText());
        } catch (NumberFormatException ex) {
            JOptionPane.showMessageDialog(
                    frame, 
                    "Bad year: " + yearTextField.getText(), 
                    "Error",
                    JOptionPane.ERROR_MESSAGE);

            return;
        }

        start.set(year,
                  monthComboBox.getSelectedIndex(),
                  daySlider.getValue());

        if (!isValidDate(start)) {
            JOptionPane.showMessageDialog(null,
                                         "The input date is invalid!", 
                                         "Error", 
                                         JOptionPane.ERROR_MESSAGE);
            return;
        }

        Date startDate = start.getTime();
        Date endDate = end.getTime();

        long startTime = startDate.getTime();
        long endTime = endDate.getTime();

        long days = (endTime - startTime) / (1000 * 60 * 60 * 24);

        JOptionPane.showMessageDialog(
                frame, 
                "You are " + days + " day" + (days == 1 ? "" : "s") + " old.", 
                "", 
                JOptionPane.INFORMATION_MESSAGE);
    }

    /**
     * Checks the validity of the input date.
     * 
     * @param  c the input date.
     * @return {@code true} only if the input date is valid.
     */
    private boolean isValidDate(Calendar c) {
        c.setLenient(false);

        try {
            c.get(Calendar.DAY_OF_MONTH);
            c.get(Calendar.MONTH);
            c.get(Calendar.YEAR);
            return true;
        } catch (Exception ex) {
            return false;
        }
    }
}

So, what do you think?

\$\endgroup\$
2
\$\begingroup\$

Comments and naming

I'd same some of your comments identifying the fields are rather inane - I can guess that private final JFrame frame; is in fact "the actual frame of this program." By contrast, some of the other comments are pretty useful, but more because the name of the field isn't as descriptive as possible - its almost always better to have your code be self explanatory (through clear formatting and logic, as well as good names) than to comment and explain it. For example dayLabel could probably be more clearly named as currentSelectedDayLabel and then remove the comment.

Likewise, anyone who is even a little familiar with Java (and even people who aren't) can guess/know that the lines

chooseDaySlider.addChangeListener(this);
computeAgeButton.addActionListener(this);

are adding listeners without a comment explaining that.

Lambda function

In your main function you use a lambda, which is fine, but you could do it even more cleanly with a method reference.

public static void main(final String... args) {
    javax.swing.SwingUtilities.invokeLater(DaysCounter::new);
}

Casting

You unnecessarily cast your JSlider to a... JSlider. Just call getValue() directly.

@Override
public void stateChanged(ChangeEvent e) {
    currentSelectedDayLabel.setText("" + chooseDaySlider.getValue());
}

actionPerformed

This part seems pretty good, however I'd say you could refactor the distinct sections into their own methods. Furthermore, the entire "days alive" calculation can be simplified. It's also a little magic number-y, but I'd say these are okay because they make sense - 1000 ms, 60 s, 60 m, 24 hr. If you wanted to make these more explicit you could, but in this case I think they would just make it harder to understand.

/**
 * Initiates the age calculation.
 *
 * @param e the button press event, ignored.
 */
@Override
public void actionPerformed(ActionEvent e) {
    Calendar start = Calendar.getInstance();
    Calendar end = Calendar.getInstance();

    int year = getYear();

    if (year == -1) return;

    start.set(year,
            monthComboBox.getSelectedIndex(),
            chooseDaySlider.getValue());

    if (!isValidDate(start)) {
        JOptionPane.showMessageDialog(null,
                "The input date is invalid!",
                "Error",
                JOptionPane.ERROR_MESSAGE);
        return;
    }

    long days = calculateNumDaysAlive(start.getTime(), end.getTime());

    JOptionPane.showMessageDialog(
            frame,
            "You are " + days + " day" + (days == 1 ? "" : "s") + " old.",
            "",
            JOptionPane.INFORMATION_MESSAGE);
}

/**
 * Gets the numeric year
 *
 * @return the year if it can be determined, otherwise -1.
 */
private int getYear() {
    try {
        return Integer.parseInt(yearTextField.getText());
    } catch (NumberFormatException ex) {
        JOptionPane.showMessageDialog(
                frame,
                "Bad year: " + yearTextField.getText(),
                "Error",
                JOptionPane.ERROR_MESSAGE);
        return -1;
    }
}

Tests

It wouldn't hurt to have some tests either, although given the relative simplicity of the calculations here it probably isn't that big of a deal.

Everything

Here is how I adapted your code in total.

package net.coderodde.gui.date;

import java.awt.Dimension;
import java.awt.GridLayout;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Calendar;
import java.util.Date;
import javax.swing.JButton;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JSlider;
import javax.swing.JTextField;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;

public class DaysCounter implements ChangeListener, ActionListener {

    private final JFrame frame;
    private final JLabel currentSelectedDayLabel;
    private final JSlider chooseDaySlider;
    private final JComboBox monthComboBox;
    private final JTextField yearTextField;
    private final JButton computeAgeButton;

    public DaysCounter() {
        this.frame = new JFrame("Age in days");
        this.currentSelectedDayLabel = new JLabel("16");
        this.chooseDaySlider = new JSlider(1, 31, 16);

        String[] months = new String[] {
                "January",
                "February",
                "March",
                "April",
                "May",
                "June",
                "Jule",
                "August",
                "September",
                "October",
                "November",
                "December",
        };

        this.monthComboBox = new JComboBox(months);
        this.yearTextField = new JTextField("1980");
        this.computeAgeButton = new JButton("Compute");

        chooseDaySlider.addChangeListener(this);
        computeAgeButton.addActionListener(this);

        currentSelectedDayLabel.setHorizontalAlignment(JLabel.CENTER);

        frame.getContentPane().setLayout(new GridLayout(5, 1));
        frame.getContentPane().add(currentSelectedDayLabel);
        frame.getContentPane().add(chooseDaySlider);
        frame.getContentPane().add(monthComboBox);
        frame.getContentPane().add(yearTextField);
        frame.getContentPane().add(computeAgeButton);

        frame.pack();

        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setResizable(false);

        // Center out the frame.
        Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();

        frame.setLocation((dim.width - frame.getWidth()) / 2,
                (dim.height - frame.getHeight()) / 2);

        frame.setVisible(true);
    }

    public static void main(final String... args) {
        javax.swing.SwingUtilities.invokeLater(DaysCounter::new);
    }

    /**
     * Change the current day of month.
     *
     * @param e the event object.
     */
    @Override
    public void stateChanged(ChangeEvent e) {
        currentSelectedDayLabel.setText("" + chooseDaySlider.getValue());
    }

    /**
     * Initiates the age calculation.
     *
     * @param e the button press event, ignored.
     */
    @Override
    public void actionPerformed(ActionEvent e) {
        Calendar start = Calendar.getInstance();
        Calendar end = Calendar.getInstance();

        int year = getYear();

        if (year == -1) return;

        start.set(year,
                monthComboBox.getSelectedIndex(),
                chooseDaySlider.getValue());

        if (!isValidDate(start)) {
            JOptionPane.showMessageDialog(null,
                    "The input date is invalid!",
                    "Error",
                    JOptionPane.ERROR_MESSAGE);
            return;
        }

        long days = calculateNumDaysAlive(start.getTime(), end.getTime());

        JOptionPane.showMessageDialog(
                frame,
                "You are " + days + " day" + (days == 1 ? "" : "s") + " old.",
                "",
                JOptionPane.INFORMATION_MESSAGE);
    }

    /**
     * Gets the numeric year
     *
     * @return the year if it can be determined, otherwise -1.
     */
    private int getYear() {
        try {
            return Integer.parseInt(yearTextField.getText());
        } catch (NumberFormatException ex) {
            JOptionPane.showMessageDialog(
                    frame,
                    "Bad year: " + yearTextField.getText(),
                    "Error",
                    JOptionPane.ERROR_MESSAGE);
            return -1;
        }
    }

    /**
     * Calculates the number of days between two dates.
     *
     * @param start the staring date.
     * @param end the ending date.
     * @return the number of days between the two dates.
     */
    private long calculateNumDaysAlive(Date start, Date end) {
        return (end.getTime() - start.getTime()) / (1000 * 60 * 60 * 24);
    }

    /**
     * Checks the validity of the input date.
     *
     * @param  c the input date.
     * @return {@code true} only if the input date is valid.
     */
    private boolean isValidDate(Calendar c) {
        c.setLenient(false);

        try {
            c.get(Calendar.DAY_OF_MONTH);
            c.get(Calendar.MONTH);
            c.get(Calendar.YEAR);
            return true;
        } catch (Exception ex) {
            return false;
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$
  • Instead of:

    currentSelectedDayLabel.setText("" + chooseDaySlider.getValue());
    

    I'd use:

    currentSelectedDayLabel.setText(Integer.toString(chooseDaySlider.getValue()));
    

    This creates just one String object (the result) instead of two (the empty and the resulting) and a StringBuilder object to append them.

    See also the accepted answer of Java int to String - Integer.toString(i) vs new Integer(i).toString().

  • I'd use:

    String.format("You are %d%s old.", days, days == 1 ? "" : "s")
    

    instead of string concatenation:

    "You are " + days + " day" + (days == 1 ? "" : "s") + " old."
    

    This shows the structure of the resulting string and the parameter types at a glance.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.