5
\$\begingroup\$

Today, I made a calculator in JFrame. The code is a little long but I'm still a beginner to Java. In the future I'm going to do some improvements on it, but here it is right now.
This is the meat of the calculator:

@SuppressWarnings("unused")
public class CalcSubClass extends JFrame {
    private static final long serialVersionUID = 1L;

JTextField numDisplay;
JButton zero;
JButton one;
JButton two;
JButton three;
JButton four;
JButton five;
JButton six;
JButton seven;
JButton eight;
JButton nine;
JButton add;
JButton subtract;
JButton multiply;
JButton divide;
JButton percent;
JButton equals;
JButton clear;
String addString;
String subString;
String multiString;
String divString;
String finalString;
double addDouble;
double subDouble;
double multiDouble;
double divDouble;
double finalDouble;
double answer;
String answerAsString;
String percentAsString;
double percentAsDouble;
double prcntAnswerAsDouble;
String prcntAnswerAsString;
JButton decimalPoint;

public static double add(double num1, double num2) {

    return num1 + num2;
}

public static double subtract(double num1, double num2) {

    return num1 - num2;
}

public static double multiply(double num1, double num2) {

    return num1 * num2;
}

public static double divide(double num1, double num2) {

    return num1 / num2;
}

public CalcSubClass() {
    super("Calculator");
    setLayout(new GridBagLayout());

    setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    setVisible(true);
    setResizable(false);

    GridBagConstraints gbc = new GridBagConstraints();

    numDisplay = new JTextField();
    numDisplay.setPreferredSize(new Dimension(130, 30));
    numDisplay.setEditable(false);
    gbc.gridx = 1;
    gbc.gridy = 0;
    gbc.gridwidth = 3;
    gbc.fill = GridBagConstraints.HORIZONTAL;
    add(numDisplay, gbc);

    zero = new JButton("0");
    gbc.gridx = 1;
    gbc.gridy = 5;
    gbc.gridwidth = 2;
    gbc.fill = GridBagConstraints.HORIZONTAL;
    add(zero, gbc);

    one = new JButton("1");
    gbc.gridx = 1;
    gbc.gridy = 2;
    gbc.gridwidth = 1;
    add(one, gbc);

    two = new JButton("2");
    gbc.gridx = 2;
    gbc.gridy = 2;
    add(two, gbc);

    three = new JButton("3");
    gbc.gridx = 3;
    gbc.gridy = 2;
    add(three, gbc);

    four = new JButton("4");
    gbc.gridx = 1;
    gbc.gridy = 3;
    add(four, gbc);

    five = new JButton("5");
    gbc.gridx = 2;
    gbc.gridy = 3;
    add(five, gbc);

    six = new JButton("6");
    gbc.gridx = 3;
    gbc.gridy = 3;
    add(six, gbc);

    seven = new JButton("7");
    gbc.gridx = 1;
    gbc.gridy = 4;
    add(seven, gbc);

    eight = new JButton("8");
    gbc.gridx = 2;
    gbc.gridy = 4;
    add(eight, gbc);

    nine = new JButton("9");
    gbc.gridx = 3;
    gbc.gridy = 4;
    add(nine, gbc);

    add = new JButton("+");
    gbc.gridx = 1;
    gbc.gridy = 1;
    add(add, gbc);

    subtract = new JButton("-");
    gbc.gridx = 2;
    gbc.gridy = 1;
    add(subtract, gbc);

    multiply = new JButton("x");
    gbc.gridx = 3;
    gbc.gridy = 1;
    add(multiply, gbc);

    divide = new JButton("÷");
    gbc.gridx = 4;
    gbc.gridy = 2;
    add(divide, gbc);

    percent = new JButton("%");
    gbc.gridx = 4;
    gbc.gridy = 1;
    add(percent, gbc);

    decimalPoint = new JButton(".");
    gbc.gridx = 3;
    gbc.gridy = 5;
    add(decimalPoint, gbc);

    equals = new JButton("=");
    gbc.gridx = 4;
    gbc.gridy = 3;
    gbc.gridheight = 3;
    gbc.fill = GridBagConstraints.VERTICAL;
    add(equals, gbc);

    clear = new JButton("AC");
    gbc.gridx = 4;
    gbc.gridy = 0;
    gbc.gridheight = 1;
    add(clear, gbc);

    zero.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "0");

        }

    });

    one.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "1");

        }

    });

    two.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "2");

        }

    });

    three.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "3");

        }

    });

    four.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "4");

        }

    });

    five.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "5");

        }

    });

    six.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "6");

        }

    });

    seven.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "7");

        }

    });

    eight.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "8");

        }

    });

    nine.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + "9");

        }

    });

    add.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            addString = numDisplay.getText();
            addDouble = Double.parseDouble(addString);
            numDisplay.setText("");

        }

    });

    subtract.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            subString = numDisplay.getText();
            subDouble = Double.parseDouble(subString);
            numDisplay.setText("");
            decimalPoint.setEnabled(true);

        }

    });

    multiply.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            multiString = numDisplay.getText();
            multiDouble = Double.parseDouble(multiString);
            numDisplay.setText("");
            decimalPoint.setEnabled(true);

        }

    });

    divide.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            divString = numDisplay.getText();
            divDouble = Double.parseDouble(divString);
            numDisplay.setText("");
            decimalPoint.setEnabled(true);
        }

    });

    percent.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            percentAsString = numDisplay.getText();
            percentAsDouble = Double.parseDouble(percentAsString);
            prcntAnswerAsDouble = percentAsDouble * 0.01;
            prcntAnswerAsString = Double.toString(prcntAnswerAsDouble);
            numDisplay.setText(prcntAnswerAsString);
        }

    });

    decimalPoint.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText(numDisplay.getText() + ".");
            String numDisplayContents = numDisplay.getText();

            if (numDisplayContents.contains(".")) {
                decimalPoint.setEnabled(false);
            }

        }

    });
    clear.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            numDisplay.setText("");
            addString = null;
            subString = null;
            multiString = null;
            divString = null;
            finalString = null;
            prcntAnswerAsString = null;
        }

    });

    equals.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            finalString = numDisplay.getText();
            finalDouble = Double.parseDouble(finalString);

            if (addString != null) {
                answer = add(addDouble, finalDouble);
                answerAsString = Double.toString(answer);
                numDisplay.setText(answerAsString);

            } else if (subString != null) {
                answer = subtract(subDouble, finalDouble);
                answerAsString = Double.toString(answer);
                numDisplay.setText(answerAsString);

            } else if (multiString != null) {
                answer = multiply(multiDouble, finalDouble);
                answerAsString = Double.toString(answer);
                numDisplay.setText(answerAsString);

            } else if (divString != null) {
                answer = divide(divDouble, finalDouble);
                answerAsString = Double.toString(answer);
                numDisplay.setText(answerAsString);

            } else {
                addString = null;
                subString = null;
                multiString = null;
                divString = null;
            }
        }

    });

  }

}

This is the main class:

public class Calculator {
    public static void main(String[] args) {

    CalcSubClass calc = new CalcSubClass();
    calc.pack();
   }

}
\$\endgroup\$
  • \$\begingroup\$ Welcome to CodeReview, Ethan. I hope you get some fine answers! \$\endgroup\$ – Legato Jul 20 '15 at 3:59
7
\$\begingroup\$

Calculators seem really popular...

I'll start with what stands out. new JButton("÷"); This likely doesn't work for you and definitely won't work everywhere, to use the Obelus you want to refer to its unicode, so rather instantiate using new JButton("\u00F7");

Instead of having all those variables for your number buttons simple create an array.

JButton[] buttons = new JButton[10];

and instantiate using a loop, which you can also leverage to add the relevant action listeners using a lambda expression

As an example of a Lambda Expression, you have a lot of places where you write this code:

addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            // content
        }
    });

this can be shortened to just

addActionListener(e -> {
 // content
});

As we see in the loop:

for (int i = 0; i < buttons.length; i++) {
  String val = Integer.toString(i)
  buttons[i] = new JButton(val);
  buttons[i].addActionListener(e -> numDisplay.setText(numDisplay.getText() + val));
}

If you're worried about positioning consider using a layout manager and placing the buttons within, e.g. JPanel numberPanel = new JPanel(new GridLayout(0, 3)); of course with that in our loop we would also call numberPanel.add(buttons[i]);

Your many static methods are very similar, you can simplify them by using an interface method also using lambda expressions.

First, defining the interface:

We know we always have two inputs with doubles as input and return value, so:

interface Equation {
    double compute(double alpha, double beta);
}

and now an enumeration to use it:

public enum Operation implements Equation {
    ADD("+", (x, y) -> x + y),
    SUBTRACT("-", (x, y) -> x - y),
    MULTIPLY("x",(x, y) -> x * y),
    DIVIDE("\u00F7", (x, y) -> x / y);

    private final Equation equation;
    private final String symbol

    Operation(String symbol, Equation equation) {
        this.symbol = symbol;
        this.equation = equation;
    }

    @Override
    public double compute(double alpha, double beta) {
        return equation.compute(alpha, beta);
    }

    @Override
    public String toString() {
        return symbol;
    }
}

You can make operator buttons like so:

JButton[] operatorButtons = new JButton[Operation.values().length];
for (int i = 0; i < operatorButtons.length; i++) {
  Operation op = Operation.values()[i];
  operatorButtons[i] = new JButton(op.toString)
  operatorButtons[i].addActionListener(e -> numDisplay.setText(op.compute(/* */));
}
\$\endgroup\$
  • \$\begingroup\$ The division symbol works for me but your right it probably won't work everywhere. And also thanks for the feedback! Now I can shorten up my code a bit! \$\endgroup\$ – Ethan Cotler Jul 20 '15 at 4:12
  • 3
    \$\begingroup\$ If I want to be snarky the Operation enum isn't actually an Equation but a binary operator, or DoubleBinaryOperator. :p +1 anyways. \$\endgroup\$ – h.j.k. Jul 20 '15 at 5:58

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.