Swing Temperature Converter

I made a simple temperature converter to serve as an introduction to swing. I used Eclipse WindowBuilder and did not attempt to make the components more modular. I'm curious as to how you guys would organize this code to improve readability.

Also, if you have any other criticism I'd love to hear it.

```
import java.awt.BorderLayout;
import java.awt.EventQueue;

import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.border.EmptyBorder;
import javax.swing.JTextField;
import java.awt.TextField;
import java.awt.FlowLayout;
import javax.swing.JTextPane;
import javax.swing.JButton;
import javax.swing.JRadioButton;
import javax.swing.JLabel;
import javax.swing.UIManager;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;
import javax.swing.ButtonGroup;

public class TempConvert extends JFrame {

private JPanel contentPane;
private final ButtonGroup buttonGroup = new ButtonGroup();

/**
* Launch the application.
*/
public static void main(String[] args) {
EventQueue.invokeLater(new Runnable() {
public void run() {
try {
TempConvert frame = new TempConvert();
frame.setVisible(true);
} catch (Exception e) {
e.printStackTrace();
}
}
});
}

/**
* Create the frame.
*/
public TempConvert() {

setTitle("Temperature Converter");
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
setBounds(0, -33, 385, 272);
contentPane = new JPanel();
contentPane.setBorder(new EmptyBorder(5, 5, 5, 5));
setContentPane(contentPane);
contentPane.setLayout(null);

JTextPane txtTempIn = new JTextPane();
txtTempIn.setBounds(12, 46, 124, 26);
contentPane.add(txtTempIn);

JRadioButton celRadio = new JRadioButton("Celcius");
buttonGroup.add(celRadio);
celRadio.setBounds(212, 34, 67, 24);
contentPane.add(celRadio);

JRadioButton fahrenRadio = new JRadioButton("Fahrenheit");
buttonGroup.add(fah

Inheritance

Do not extend any class unless you you have an "is a" relationship and add new behavior to the extended class.

You extend JFrame without adding Frame related behavior, you just configure it.


I don't understand the objection to extending JFrame here. The program needs a top level container.

Right, but you program does not need to inherit form JFrame to get one:

public class TempConvert {

private JPanel contentPane;
private final ButtonGroup buttonGroup = new ButtonGroup();

/**
 * Launch the application.
 */
public static void main(String[] args) {
    EventQueue.invokeLater(new Runnable() {
        public void run() {
            try {
                TempConvert frame = new JFrame();
                new TempConvert(frame);
                frame.setVisible(true);
            } catch (Exception e) {
                e.printStackTrace();
            }
        }
    });
}

/**
 * Create the frame.
 */
public TempConvert(JFrame frame) {

    frame.setTitle("Temperature Converter");
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    frame.setBounds(0, -33, 385, 272);
    frame.contentPane = new JPanel();
    frame.contentPane.setBorder(new EmptyBorder(5, 5, 5, 5));
    frame.setContentPane(contentPane);
    frame.contentPane.setLayout(null);

Layouting

When using NULL layout you throw away any convenience Layoutmanager could give you. Your UI will be bound to the "ideal" Frame size. The problem is that the definition of "ideal" changes with every user and every device your program runs on.

Magic Numbers

Because of your wrong interpretation of "Flexibility" that results in using the NULL Layout you need to specify bound of your buttons. And you do this by using random literal numbers (from the readers perspective). You should at least define constants for them.

Code duplication

Your code has lots of duplicated code.

You create a bunch of buttons. you could extract the creation of a single button to a parameterized method.

Also in your anonymous Listener class the try/catch block is duplicated. Here you could simply move one of then around the whole if and delete the other one.

StackExchange Code Review Q#147714, answer score: 5