patternjavaMinor
Celsius and Fahrenheit Conversion
Viewed 0 times
andconversionfahrenheitcelsius
Problem
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
```
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.getT
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.getT
Solution
First, some style nitpicks: Throughout, it would be better to have more descriptive, spelled out, camelCased variable names. As a reader,
Class names should be capitalized (
Since
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
At the very end, you're setting
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.
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.
Context
StackExchange Code Review Q#155146, answer score: 3
Revisions (0)
No revisions yet.