Java Calculator

I tried to make a calculator in Java and have added some stuff. I'm a beginner and have learned yesterday how to make buttons.

```
import java.awt.Dimension;
import javax.swing.*;
import java.awt.Toolkit;
import java.awt.event.*;

public class JavaCalculatorCopy extends JFrame{

JPanel Panel = new JPanel();
JTextArea Input = new JTextArea(1,16);
JTextArea Output = new JTextArea(1,16);
JTextArea Debug = new JTextArea(5,16);
JButton Button0 = new JButton("0");
JButton Button1 = new JButton("1");
JButton Button2 = new JButton("2");
JButton Button3 = new JButton("3");
JButton Button4 = new JButton("4");
JButton Button5 = new JButton("5");
JButton Button6 = new JButton("6");
JButton Button7 = new JButton("7");
JButton Button8 = new JButton("8");
JButton Button9 = new JButton("9");
JButton ButtonDecimalPoint = new JButton(".");
JButton ButtonAns = new JButton("Ans");
JButton ButtonMakeNegative = new JButton("(-)");
JButton ButtonPlus = new JButton("+");
JButton ButtonMinus = new JButton("-");
JButton ButtonTimes = new JButton("*");
JButton ButtonDividedBy = new JButton("/");
JButton ButtonSquared = new JButton("^2");
JButton ButtonPower = new JButton("^");
JButton ButtonSquareRoot = new JButton("√");
JButton ButtonEquals = new JButton("=");
JButton ButtonReset = new JButton("Reset");
String InputString;
String FirstValue = new String();
String SecondValue = new String();
int whatOperator = 20;
double outputNumber;
double AnsValue;
double FirstNumber;
double SecondNumber;
boolean OutputInUse = false;

public static void main(String[] args){

new JavaCalculatorCopy();

}

public JavaCalculatorCopy(){

SetDebugScreen();
this.setSize(220, 390);
Toolkit tk = Toolkit.getDefaultToolkit();
Dimension dim = tk.getScreenSize();
int xPos = (dim.width / 2)

This is more to tackle than would be easy to cover in a single answer.

Small items.

There are a couple of small items that are a problem.

-
you are using capitalized names for variables.... this is against the coding style for Java. Variable names should always start with a lower-case letter. For example, your Button7 should be button7.

-
if(SecondValue.equals("")|SecondValue.equals("-")) uses the bitwise OR operator (|) instead of the logical OR operator ||. This produces the same effective result, in this instance, except that || is a short-circuit operator (if the first condition is true, it does not even evaluate the subsequent conditions), whereas the | operator will evaluate both sides of the expression. The problem is most apparent in situations like:

if (mystring == null || mystring.length() == 0) {
    return "Foo";
}

the above expression will return "Foo" for a null mystring, but the situation:

if (mystring == null | mystring.length() == 0) {
    return "Foo";
}

will throw a null-pointer exception for a null mystring

The use of | between boolean expressions is very, very unusual, and, unless it is doing bitwise logic, I would always consider it a bug.

-
The application allows you to create broken calculations (if you are a broken user, like me). For example, entering 2 and ^2 and = causes a NumberFormatException.

-
you are not taking in to account the thread-model in the Swing framework. All your work is being done on the EventDispatchThread. In this instance, the work you are doing is relatively trivial, and probably will not impact the user experience, but, it is a really, really bad habit to start to learn... use the right threads for the job.

-
your layout in your manager is not quite right.... I can't see the actual problem in the code, but visually, the buttons are not quite lining up vertically.... You should be using the GridLayout?

Using an appropriate structure

OK, that's some minor stuff to start with. The real issue, as I am sure you are aware, is that you have a lot of code duplication.

I would strongly recommend that you separate out the display logic from the calculation engine. Right now you have a single, large, class that does both.

The standard way to break up your program would be to use something like the Model-View-Controller (MVC) system for user interaction. There is a relatively good description of the process in this Java documentation.

The basic structure would be something like:

-
A calculator Model class. This class contains just the mathematical guts of your application. This is your business logic. It has methods like:

• digit(int value)

• reset

• equals

• square

• divide

• addPropertyChangeListener(Listener ....)

-
A calculator View class. This will have all the swing components, buttons, etc. It is what controls the display, and accepts user input. Complex designs will make the view an interface, and then have multiple implementations of the view (one for a web-site, one for a console, one for a GUI, one for an app, etc.).

-
A calculator Controller class. This class ties the view(s) and the engine together.

A big part of this system is the use of listeners (and interfaces). The PropertyChangeListener is how the model communicates with the Controller.... the controller then uses those events to trigger changes in the view(s).

The View

Most of your code duplication is related to the view. I strongly recommend that you create an enum called 'CalcButton' (or similar), and this enum looks something like:

public enum ButtonType {
    DIGIT, OPERATOR, CONTROL;
}

public enum CalcButton {

    ZERO("0", ButtonType.DIGIT),
    ONE("1", ButtonType.DIGIT),
    .....
    NINE("9", ButtonType.DIGIT),
    PLUS("+", ButtonType.OPERATOR),
    .....
    EQUALS("=", ButtonType.CONTROL),
    RESET("reset", ButtonType.CONTROL);

    private final JButton myjbutton;
    private final ButtonType mytype;

    private CalcButton(String label, ButtonType btype) {
        myjbutton = new JButton(label);
        mytype = btype;
    }

    public final JButton getJButton() {
        return myjbutton;
    }

    public final ButtonType getButtonType() {
        return mytype;
    }

}

The above is just to give you some ideas to start with..... but, with the above enum, consider the following:

CalcButton[][] calclayout = new CalcButton[][] {

    {CalcButton.SEVEN, CalcButton.EIGHT, CalcButton.NINE, CalcButton.PLUS},
    {CalcButton.FOUR, CalcButton.FIVE, CalcButton.SIX, CalcButton.MINUS},
    .....

}

JPanel panel = new JPanel(...);
// set up appropriate Layout Manager....

for (int row = 0; row < calclayout.length; row++) {
    for (int col = 0; col < calclayout[row].length; col++) {
        CalcButton b = calclayout[row][col];
        panel.add(b.getJButton());
        b.getJButton().setActionListener(this);
    }
}

Conclusion

Right now, your code could do with a major refactor and sh

StackExchange Code Review Q#41019, answer score: 11