3
\$\begingroup\$

Today is my first time working with GUI, so I'm really interested in the best ways of handling various events to get the best habits from the jump. Most of the GUI-related code for this project was generated by Netbeans's layout manager, although I tried to rip out as many of the meta tags that I could, did imports where it helped cut down on code, and put the heavily auto-generated code at the bottom.

My biggest question is with the tempsComboBoxItemStateChanged method. Is there a better way of having the JComboBox change the program around? Right now I have a separate "logic class" handle everything, and this just creates a brand new object initialized with the updated type it'll be converting from.

Also, is there a better way of having multiple events cause the same action than factoring out and re-using a standalone method? When the user hits Enter on the textbox or button, or clicks the Convert button, I use the convertTemp method so I don't have duplicate code. It seems like there must be a better way.

TempConverterGUI.java:

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

public class TempConverterGUI extends JFrame {
    private TempConverter converter;

    public TempConverterGUI() {
        converter = new TempConverter(TempConverter.TEMP_TYPE.Celsius);

        initComponents();
    }




    private void convertButtonActionPerformed(ActionEvent evt) {
        convertTemp();
    }

    private void tempTextFieldMouseClicked(MouseEvent evt) {
        tempTextField.setText("");
    }

    private void tempTextFieldKeyReleased(KeyEvent evt) {
        if (evt.getKeyCode() == KeyEvent.VK_ENTER) {
            convertTemp();
        }
    }

    private void convertButtonKeyReleased(KeyEvent evt) {
        if (evt.getKeyCode() == KeyEvent.VK_ENTER) {
            convertTemp();
        }
    }

    private void tempsComboBoxItemStateChanged(ItemEvent evt) {
        String tempStr = tempsComboBox.getSelectedItem().toString();

        tempTextField.setText("Temperature");
        if (tempStr.equals("Convert Celsius to Fahrenheit")) {
            converter = new TempConverter(TempConverter.TEMP_TYPE.Celsius);
        }
        else if (tempStr.equals("Convert Fahrenheit to Celsius")) {
            converter = new TempConverter(TempConverter.TEMP_TYPE.Fahrenheit);
        }
        else {
            throw new TempConverter.TempTypeDoesntExistException("Temperature type from combo box isn't supported.");
        }

        convertFromLabel.setText(converter.fromTempTypeStr());
        convertToLabel.setText(converter.toTempTypeStr());
        tempTextField.setToolTipText("Enter a temperature in degrees " + converter.fromTempTypeStr());

    }

    private void convertTemp() {
        double fromTemp = 0.0;
        int toTemp = 0;
        boolean validInput = false;

        // Parse degrees Celsius asa double and convert to Fahrenheit.
        try {
            fromTemp = new Scanner(tempTextField.getText()).nextDouble();
            validInput = true;
        }
        catch (java.util.InputMismatchException e) {
            System.out.println("**ERROR**: Entered temperature isn't valid.");
        }
        catch (java.util.NoSuchElementException e) {
            System.out.println("**ERROR**: No temperature entered.");
        }


        if (validInput) {
            toTemp = converter.convertTemp(fromTemp);
            convertToLabel.setText(toTemp + " " + converter.toTempTypeStr());
        }
        else {
            convertToLabel.setText(converter.toTempTypeStr());
        }

        pack();
    }


    /**
     * This method is called from within the constructor to initialize the Netbeans form.
     */
    private void initComponents() {

        convertFromLabel = new JLabel();
        convertToLabel = new JLabel();
        convertButton = new JButton();
        tempTextField = new JTextField();
        tempsComboBox = new JComboBox();

        setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        setTitle("Celsius Converter");

        convertFromLabel.setLabelFor(convertFromLabel);
        convertFromLabel.setText("Celsius");

        convertToLabel.setText("Fahrenheit");

        convertButton.setText("Convert");
        convertButton.setCursor(new Cursor(Cursor.HAND_CURSOR));
        convertButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent evt) {
                convertButtonActionPerformed(evt);
            }
        });
        convertButton.addKeyListener(new KeyAdapter() {
            public void keyReleased(KeyEvent evt) {
                convertButtonKeyReleased(evt);
            }
        });

        tempTextField.setText("Temperature");
        tempTextField.setToolTipText("Enter a temperature in degrees Celsius");
        tempTextField.addMouseListener(new MouseAdapter() {
            public void mouseClicked(MouseEvent evt) {
                tempTextFieldMouseClicked(evt);
            }
        });
        tempTextField.addKeyListener(new KeyAdapter() {
            public void keyReleased(KeyEvent evt) {
                tempTextFieldKeyReleased(evt);
            }
        });

        tempsComboBox.setMaximumRowCount(2);
        tempsComboBox.setModel(new DefaultComboBoxModel(new String[] { "Convert Celsius to Fahrenheit", "Convert Fahrenheit to Celsius" }));
        tempsComboBox.addItemListener(new ItemListener() {
            public void itemStateChanged(ItemEvent evt) {
                tempsComboBoxItemStateChanged(evt);
            }
        });

        GroupLayout layout = new GroupLayout(getContentPane());
        getContentPane().setLayout(layout);
        layout.setHorizontalGroup(
            layout.createParallelGroup(GroupLayout.Alignment.LEADING)
            .addGroup(layout.createSequentialGroup()
                .addContainerGap()
                .addGroup(layout.createParallelGroup(GroupLayout.Alignment.LEADING)
                    .addGroup(layout.createSequentialGroup()
                        .addComponent(convertButton)
                        .addPreferredGap(LayoutStyle.ComponentPlacement.RELATED)
                        .addComponent(convertToLabel)
                        .addGap(0, 0, Short.MAX_VALUE))
                    .addGroup(layout.createSequentialGroup()
                        .addGroup(layout.createParallelGroup(GroupLayout.Alignment.LEADING)
                            .addGroup(layout.createSequentialGroup()
                                .addComponent(tempTextField, GroupLayout.PREFERRED_SIZE, GroupLayout.DEFAULT_SIZE, GroupLayout.PREFERRED_SIZE)
                                .addPreferredGap(LayoutStyle.ComponentPlacement.RELATED)
                                .addComponent(convertFromLabel))
                            .addComponent(tempsComboBox, GroupLayout.PREFERRED_SIZE, GroupLayout.DEFAULT_SIZE, GroupLayout.PREFERRED_SIZE))
                        .addContainerGap(GroupLayout.DEFAULT_SIZE, Short.MAX_VALUE))))
        );

        layout.linkSize(SwingConstants.HORIZONTAL, new Component[] {convertButton, tempTextField});

        layout.setVerticalGroup(
            layout.createParallelGroup(GroupLayout.Alignment.LEADING)
            .addGroup(GroupLayout.Alignment.TRAILING, layout.createSequentialGroup()
                .addContainerGap()
                .addComponent(tempsComboBox, GroupLayout.PREFERRED_SIZE, GroupLayout.DEFAULT_SIZE, GroupLayout.PREFERRED_SIZE)
                .addPreferredGap(LayoutStyle.ComponentPlacement.RELATED)
                .addGroup(layout.createParallelGroup(GroupLayout.Alignment.BASELINE)
                    .addComponent(tempTextField, GroupLayout.PREFERRED_SIZE, GroupLayout.DEFAULT_SIZE, GroupLayout.PREFERRED_SIZE)
                    .addComponent(convertFromLabel))
                .addPreferredGap(LayoutStyle.ComponentPlacement.RELATED)
                .addGroup(layout.createParallelGroup(GroupLayout.Alignment.BASELINE)
                    .addComponent(convertButton)
                    .addComponent(convertToLabel))
                .addContainerGap(GroupLayout.DEFAULT_SIZE, Short.MAX_VALUE))
        );

        pack();
    }


    /**
     * @param args Command-line arguments aren't currently supported
     */
    public static void main(String args[]) {
        /* Set the Nimbus look and feel */
        /* If Nimbus (introduced in Java SE 6) is not available, stay with the default look and feel.
         * For details see http://download.oracle.com/javase/tutorial/uiswing/lookandfeel/plaf.html
         */
        try {
            for (UIManager.LookAndFeelInfo info
                    : UIManager.getInstalledLookAndFeels())
            {
                if ("Nimbus".equals(info.getName())) {
                    UIManager.setLookAndFeel(info.getClassName());
                    break;
                }
            }
        } catch (ClassNotFoundException | InstantiationException | IllegalAccessException
                    | UnsupportedLookAndFeelException ex)
        {
            java.util.logging.Logger.getLogger(TempConverterGUI.class.getName())
                                        .log(java.util.logging.Level.SEVERE, null, ex);
        }

        /* Create and display the form */
        EventQueue.invokeLater(() -> {
            new TempConverterGUI().setVisible(true);
        });
    }

    // Variables declaration - do not modify
    private JButton convertButton;
    private JLabel convertFromLabel;
    private JLabel convertToLabel;
    private JTextField tempTextField;
    private JComboBox tempsComboBox;
    // End of variables declaration
}

TempConverter.java:

public class TempConverter {
    public static enum TEMP_TYPE {
        Celsius, Fahrenheit;
    }

    private final static TEMP_TYPE DEFAULT_TEMP_TYPE = TEMP_TYPE.Celsius;
    private final TEMP_TYPE convertingFrom, convertingTo;

    public TempConverter(TEMP_TYPE convertFrom) {
        convertingFrom = convertFrom;

        if (convertFrom == TEMP_TYPE.Celsius) {
            convertingTo = TEMP_TYPE.Fahrenheit;
        }
        else if (convertFrom == TEMP_TYPE.Fahrenheit) {
            convertingTo = TEMP_TYPE.Celsius;
        }
        else {
            convertingTo = DEFAULT_TEMP_TYPE;
        }
    }



    public int convertTemp(double fromTemp) {
        int toTemp = 0;

        if (convertingFrom == TEMP_TYPE.Fahrenheit) {
            toTemp = (int)(Math.round((fromTemp - 32) * (5.0 / 9.0)));
        }
        else if (convertingFrom == TEMP_TYPE.Celsius) {
            toTemp = (int)(Math.round(fromTemp * 1.8 + 32));
        }
        else {
            throw new TempTypeDoesntExistException("Temperature type isn't supported.");
        }

        return toTemp;
    }



    public TEMP_TYPE fromTempType() {
        return convertingFrom;
    }

    public TEMP_TYPE toTempType() {
        return convertingTo;
    }

    public String fromTempTypeStr() {
        return convertingFrom.toString();
    }

    public String toTempTypeStr() {
        return convertingTo.toString();
    }




    public static class TempTypeDoesntExistException extends IndexOutOfBoundsException {
        public TempTypeDoesntExistException() { }

        public TempTypeDoesntExistException(String message) {
            super(message);
        }
    }
}
|improve this question
\$\endgroup\$
1
\$\begingroup\$

UI/UX

try {
    fromTemp = new Scanner(tempTextField.getText()).nextDouble();
    validInput = true;
}
catch (java.util.InputMismatchException e) {
    System.out.println("**ERROR**: Entered temperature isn't valid.");
}
catch (java.util.NoSuchElementException e) {
    System.out.println("**ERROR**: No temperature entered.");
}

Since you are using a Swing-based UI, displaying such errors through the console breaks the user interaction. You should consider displaying error messages using the appropriate dialog boxes too.

Comments

I'm not sure if your comment about using Nimbus (with a reference to Java 6, when you are clearly using Java 8 already) really belongs there... Also,

// Variables declaration - do not modify
private JButton convertButton;
private JLabel convertFromLabel;
private JLabel convertToLabel;
private JTextField tempTextField;
private JComboBox tempsComboBox;
// End of variables declaration

Do you need the two-liner comments here? It's quite clear these are variable declarations, and 'do not modify' seems superfluous.

Java 8, and Swing vs AWT

(both are related by the understanding of the language features you have here)

You are using Swing-based UIs, but interestingly you are coupling them with AWT's mechanism of event-handling. I am not sure if this pairing is intentional. On this topic, you can also consider using SwingUtilities.invokeLater(runnable):

public static void main(String args[]) {
    setLookAndFeel();
    SwingUtilities.invokeLater(TempConverterGUI::new);
}

This encapsulates the look-and-feel setting inside setLookAndFeel(), and uses Java 8's method reference TempConverterGUI::new as a similar lambda expression to what you have. You will need to do setVisible(true) inside the constructor, though.

TempConverter

It's good that you have used an enum to represent both temperature scales, though the Java naming convention is to continue using PascalCase for the enum name and SHOUTY_CAPS for the values. That means you can also consider writing it as such:

enum TemperatureScale {
    CELSIUS, FAHRENHEIT;
}

Instead of putting the logic inside TempConverter.convertTemp(double), you may want to consider abstracting them to be part of the TemperatureScale enum. This brings the logic 'closer' to where it is used with, i.e. the base/target scales. For example:

enum TemperatureScale {
    CELSIUS {
        @Override
        double convertTo(TemperatureScale target, double value) {
            if (Objects.requireNonNull(target) == this) {
                return value;
            }
            // return calculated value
        }
    }, FAHRENHEIT {
        @Override
        double convertTo(TemperatureScale target, double value) {
            // ...
        }
    };

    abstract double convertTo(TemperatureScale target, double value);
}

In this case, your enum type is no longer used as a 'flag', instead given an arguably better usage for performing the calculation.

|improve this answer
\$\endgroup\$
  • \$\begingroup\$ This is really helpful! Regarding the comments, the ones you referenced were auto-generated, and I should have removed them from the post. The same goes for the coupling with AWT/Swing, and Netbeans "protects" the "form"-generated code so it can't be edited. I also hadn't thought to include methods inside the enum; I keep forgetting I can assign it its own methods just like any other class. :) I had the enum names in Naming Case so they could be easily referenced by getName(), but I'll update it to be SHOUTY_CAPS and override the getName() to convert it to how it should look. .......... \$\endgroup\$ – BrainFRZ Oct 26 '15 at 18:06
  • \$\begingroup\$ ..... Is it normal to do logic calculations inside an enum though? This would really cut down on code, but it doesn't quite sit right with me semantically. Aren't enums just meant to be a limiter of a type's domain and nothing more? I understand "helper" methods, such as converters from ints to an enum value, or changing the name like I mentioned before, but program logic seem to be a totally separate entity. This might just be the novice coming out of me, so if this is normal, I'll certainly embrace it! Thanks again! \$\endgroup\$ – BrainFRZ Oct 26 '15 at 18:09
  • 1
    \$\begingroup\$ An enum is ultimately still a class, so I would say treat it like any other class in your OOP design. If there are certain calculations that seem 'right' to belong to a class, then I don't see issues incorporating it into the enum too. :) For example, if you look at Oracle's 101 on enums, the variables required for calculating a planet's gravitational pull, and the calculation itself, are part of the enum definition. \$\endgroup\$ – h.j.k. Oct 26 '15 at 23:33
  • \$\begingroup\$ BTW it's toString() that you can override, not getName() of an enum as that is a final method. :) Thanks for explaining the reasons behind my observations... \$\endgroup\$ – h.j.k. Oct 26 '15 at 23:48
  • 1
    \$\begingroup\$ I did more thinking about it and looking around, and it actually seems like using enums like you suggest is a wonderful idea because it's semantically defining a subset of an otherwise unlimited number of the class type being defined in the enum, which is great. Thanks for the tip!! :) \$\endgroup\$ – BrainFRZ Oct 27 '15 at 12:51
0
\$\begingroup\$

I've had similar thoughts about having multiple swing items have the same action listener, but I think its only possible if you set the custom code in the properties field on the design editor, or add your own code that adds your listener to all your swing items.

|improve this answer
\$\endgroup\$
  • \$\begingroup\$ Hmm, could you point me to a link that would teach me how to do that? I've only been able to find how to add regular listener methods. \$\endgroup\$ – BrainFRZ Oct 26 '15 at 18:25

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.