Celsius and Fahrenheit Conversion

Question

This is a school project submitted last year, and I've been looking over it and trying to figure out what I could improve on. I know I used celsiusInput and fahrenheitOutput the wrong way around in Fahrenheit conversion, that was just laziness at that point and not wanting to break something before handing it in. I would like some advice on improvements I can make, or bad habits I should stay away from in the future.

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

public class window{
    static boolean refresh, celsius=true, fahrenheit=true;
    protected static JTextField celsiusInput;
    protected static JTextArea fahrenheitOutput;

    private static void createAndShowGUIfahrenheit() {
        celsiusInput = new JTextField(8);
        fahrenheitOutput = new JTextArea(1, 8);
        fahrenheitOutput.setEditable(false);        

        JFrame window = new JFrame("Temperature Conversion");
        JButton convert = new JButton ("Convert");
        JLabel label = new JLabel("Fahrenheit to Celsius");

        JPanel convert1 = new JPanel();
            convert1.add(convert);
        JPanel celsius = new JPanel();
            celsius.add(celsiusInput);
        JPanel fahrenheit = new JPanel();
            fahrenheit.add(fahrenheitOutput);


        window.setSize(350, 150);
        window.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
        window.setVisible(true);
        window.setResizable(false);
        window.setLocation(1235, 465);

        window.add(convert1, BorderLayout.CENTER);
        window.add(celsius, BorderLayout.WEST);
        window.add(fahrenheit, BorderLayout.EAST);
        window.add(label, BorderLayout.NORTH);

        convert.addActionListener (new ActionListener(){
            public void actionPerformed(ActionEvent ae){
                refresh = !refresh;

                System.out.println("toggled");

                if(refresh==true){

                    float celTemp = Float.parseFloat(celsiusInput.getText());
                    float T = celTemp - 32;
                    T = T * 5/9;
                    fahrenheitOutput.setText(String.valueOf(T));
                    System.out.println("true");                 
                    refresh=false;
                }
            }
        });
    }

    private static void createAndShowGUIcelsius() {
        celsiusInput = new JTextField(8);
        fahrenheitOutput = new JTextArea(1, 8);
        fahrenheitOutput.setEditable(false);        

        JFrame window = new JFrame("Temperature Conversion");
        JButton convert = new JButton ("Convert");
        JLabel label = new JLabel("Celsius to Fahrenheit");


        JPanel convert1 = new JPanel();
            convert1.add(convert);
        JPanel celsius = new JPanel();
            celsius.add(celsiusInput);
        JPanel fahrenheit = new JPanel();
            fahrenheit.add(fahrenheitOutput);


        window.setSize(350, 150);
        window.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
        window.setVisible(true);
        window.setResizable(false);
        window.setLocation(385, 465);

        window.add(convert1, BorderLayout.CENTER);
        window.add(celsius, BorderLayout.WEST);
        window.add(fahrenheit, BorderLayout.EAST);
        window.add(label, BorderLayout.NORTH);

        convert.addActionListener (new ActionListener(){
            public void actionPerformed(ActionEvent ae){
                refresh = !refresh;

                System.out.println("toggled");

                if(refresh==true){

                    float celTemp = Float.parseFloat(celsiusInput.getText());
                    float T = celTemp * 9/5;
                    T = T + 32;
                    fahrenheitOutput.setText(String.valueOf(T));
                    System.out.println("true");     
                    refresh=false;
                }
            }
        });
    }

    public static void main( String[] args){

        JFrame mainwindow = new JFrame("Temperature Conversion");
        JButton quitbtn = new JButton("Exit");
        JButton cels = new JButton ("Celsius to Fahrenheit");
        JButton fahr = new JButton ("Fahrenheit to Celsius");
        JLabel label = new JLabel("Choose an option");


        mainwindow.setSize(350, 150);
        mainwindow.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        mainwindow.setVisible(true);
        mainwindow.setResizable(false);
        mainwindow.setLocation(785, 265);

        mainwindow.add(quitbtn, BorderLayout.SOUTH);
        mainwindow.add(cels, BorderLayout.WEST);
        mainwindow.add(fahr, BorderLayout.EAST);
        mainwindow.add(label, BorderLayout.NORTH);

        quitbtn.addActionListener(e -> System.exit(0));


        cels.addActionListener (new ActionListener(){
            public void actionPerformed(ActionEvent ae){
                celsius = false;
                System.out.println("CELSIUS TRIGGERED");                
                if (celsius==false){
                    createAndShowGUIcelsius();
                    System.out.println("new window celsius");
                    celsius = true;
                }
            }
        });

        fahr.addActionListener (new ActionListener(){
            public void actionPerformed(ActionEvent ae){
                fahrenheit = false;
                System.out.println("FAHRENHEIT TRIGGERED");
                if (fahrenheit==false){
                    createAndShowGUIfahrenheit();
                    System.out.println("new window fahrenheit");
                    fahrenheit = true;
                }
            }
        });
    }
}

Answer 1

First, some style nitpicks: Throughout, it would be better to have more descriptive, spelled out, camelCased variable names. As a reader, quitButton is so much easier to understand than quitbtn.

Class names should be capitalized (window -> Window). It's also not great to have a local variable the same name as your class. I'm surprised that didn't give you a warning.

Since refresh==true will always evaluate to refresh, you can simplify if (refresh==true) to if (refresh).

My knowledge of Java is not great, so this next part will be largely a vague suggestion, but it should be clear what I'm trying to say. You have a lot of repeated code, which is a flag for refactoring. In particular, you could have a TemperatureConverter class, with FarhenheitToCelsiusConverter and CelsiusToFahrenheitConverter as subclasses. Then you could have an abstract convertTemperature method that they override. The location could be passed in as a parameter. This follows the DRY Principle.

At the very end, you're setting fahrenheit/celsius to false, and then checking if they're false... I'm not sure I understand what purpose the flags serve at all. Are they needed?

On a higher-level, this probably wouldn't be great if you need to add Kelvin or Rankine units. I'm not going to try to give UX advice, but if that's something you want to worry about, there are plenty of online samples to look at for inspiration.

Answer 2

I don't have much experiance with Java but there is some general advice that I can offer.

---

At the moment you have no reusable components. The logic for converting temperatures is bueried inside your GUI code.

If you wanted to adapt this program to run from a CLI or be used within another program it would currently be impossible.

I would recomend that you extract the handeling of temperature conversion into its own class, maybe Temperature?

public class Temperature {

    private float temperature; // the temperature stored in Celsuis

    public function Temperature(float temperature, String unit) {
        // Convert from the current unit to Celsius
    }

    public float inFahrenheit() {
        // perform conversion here
    } 
}

This would allow you to easily add conversion to other units.

---

I don't know anything about the GUI classes that you are using but you seem to be repeating yourself when building them. Could you extend JFrame to build the entire component, or write your own GUI class?

Something like:

public class TemperatureWindow {
    public function TemperatureWindow() {
        // build up a the frame and button and input and so on 
        // store them as member variables
    }

    public void setInputText(String text){...}

    public void setListener(/* whatever */){...}
}