Simple Calculator in Java and Swing

Question

I'm new at this and I'm almost done with learning basic Java. I made this just to see if I could do it. I just want to know if there's anything I can do to make it better.

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import static java.lang.Double.*;

class Calculator {
    private JFrame frame = new JFrame("Simple Calculator");
    private JPanel panel = new JPanel();
    private JTextField inputOne = new JTextField(5);
    private JTextField inputTwo = new JTextField(5);
    private JTextField output = new JTextField(20);
    private String[] symbols = {"\u002b", "\u2212", "\u00f7", "\u00d7"};
    private JComboBox list = new JComboBox(symbols);
    private JButton calculateButton = new JButton("Calculate");

    public static void main(String[] args) {
        Calculator calculator = new Calculator();
        calculator.run();
    }

    public void run() {
        buttonListener buttonListener = new buttonListener();

        calculateButton.addActionListener(buttonListener);
        calculateButton.setFocusPainted(false);
        calculateButton.setBackground(Color.WHITE);
        calculateButton.setForeground(Color.BLACK);

        inputOne.addActionListener(buttonListener);
        inputTwo.addActionListener(buttonListener);
        output.setEditable(false);

        list.setBackground(Color.GRAY);
        list.setForeground(Color.WHITE);
        list.setEditable(false);
        list.setFocusable(false);

        panel.setBackground(Color.DARK_GRAY);
        panel.add(inputOne);
        panel.add(list);
        panel.add(inputTwo);
        panel.add(output);

        frame.getContentPane().add(BorderLayout.SOUTH, calculateButton);
        frame.getContentPane().add(BorderLayout.CENTER, panel);
        frame.setSize(300, 200);
        frame.setVisible(true);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }

    static double calculate(double a, double b, String c) {
        double result = 0;

        if (c.equals("\u002b")) {
            result = a + b;
        } else if (c.equals("\u2212")) {
            result = a - b;
        } else if (c.equals("\u00f7")) {
            result = a / b;
        } else if (c.equals("\u00d7")) {
            result = a * b;
        }

        return result;
    }

    class buttonListener implements ActionListener {
        public void actionPerformed(ActionEvent e) {
            if (!inputOne.getText().equals("") & !inputTwo.getText().equals("")) {
                double numberOne = (parseDouble(inputOne.getText()));
                double numberTwo = (parseDouble(inputTwo.getText()));

                String function = (String) list.getSelectedItem();

                output.setText("" + Calculator.calculate(numberOne, numberTwo, function));
            }

        }
    }
}

Answer 1

pros

• you don't inherit from JFrame (as too many tutorials out there do...)
• you (almost) follow Java naming conventions
• your variable names almost express the meaning of the values they hold

cons

• you break Java Naming conventions with the ActionListener implementation.
• frame.setVisible(true); is not the last line of frame configuration.
• you use literal strings instead of constants with meaningful names
• you use a static method.
• you use an if/else cascade instead using polymorphism to find the calculation.

suggestions

Naming conventions

• start class names with an uppercase letter.
• start names of variables and methods with lower case letter
• chose nouns to name classes and variables
• start names of methods with a verb maybe followed by nouns.

position of frame.setVisible(true);

The problem with that is that modifications to the Layout may not be visible if dome after frame.setVisible(true);. Therefore you should get used to write it ata position where you cannot write something behind it by accident.

One way to asure this is to do the configuration in a method where you pass the JFrame as parameter and call frame.setVisible(true); after that method like this:

   public static void main(String[] args) {        
        JFrame frame = new JFrame("Simple Calculator");
        Calculator calculator = new Calculator();
        calculator.configureGui(frame);
        frame.setVisible(true);
   }

variable names

some of your names are already good, but others need improvement
e.g.: list -> operators

static method

You need a static method for a technical reason. This is because you don't know (yet) the differences between inner classes, anonymous inner classes and top level classes in same file and their consequences for the visibility of variables and methods.

In your case the Action listener should be an anonymous inner class so that is has full access to the members (variables and methods) of the containing class:

public void run() {
    ActionListener buttonListener = new ActionListener(){    
        public void actionPerformed(ActionEvent e) {
            if (!inputOne.getText().equals("") & !inputTwo.getText().equals("")) {
                double numberOne = (parseDouble(inputOne.getText()));
                double numberTwo = (parseDouble(inputTwo.getText()));    
                String function = (String) list.getSelectedItem();
                // calculate is accessible as member from here...    
                output.setText("" + calculate(numberOne, numberTwo, function));
            }
        }
     };
     // ...

But on the other hand there is the MVC (or MVVC) pattern to separate User Interface (UI = View) the data structure (model) and the business logic (controller). This pattern works with (public) top level classes accessing public methods.

if / else cascade -> polymorphism

To resolve this we need to introduce an interface, that all the operations can implement so that we can do the same call on any operation:

interface Operation {
   double  calculate(double term1, double term2);
}

This could be declared within the Calculator class just as you did with your ActionListener implementation.

Then we need to create and store implementations of this interface and map them with their symbols. The best solution for this is: --- a Map. Therefor we change your variable symbols to a HashMap:

 private Map<String,Operation> symbols = new HashMap<>();

In your configuration method we initialize this Map.

// literals replaced with constants...
symbols.put(PLUS, new Operation(){
     public double calculate(double summand1, double summand2){
        return summand1+summand2;
     }
});
// same as Java8 Lambda
symbols.put(MINUS, (minuend, subtrahend) -> minuend-subtrahend);
//...

having this your calculate method would change to this:

 double calculate(double a, double b, String symbol) {
   if(symbols.containsKey(symbol))
     return symbols.get(symbol).calculate(a,b);
   else
     throw new IllegalOperationException("symbol "+symbol+" unknown!");
 }

isn't that nice?