Java calculator using Swing

Question

This is a simple calculator where the user can type out the calculation and hit enter, and the calculator would determine if it is a valid calculation or not by attempting to carry out the calculation. If it fails, then an error message is printed to the screen. If it is a valid calculation, the calculation is carried out.

Is the code well written?
Is it easily extendable i.e. implementing a button that switches between normal operator panel scientific one?
Are there any problems with coupling and cohesion?
What could be done to make the code more robust?

Calculator engine:

public class Calculator {
    @SuppressWarnings("unused")
    private CalculatorGui gui;


    private static final int BRACKETS = 5;
    @SuppressWarnings("unused")
    private static final int ORDERS = 4; //for future expansion;
    private static final int DIVISION = 3;
    private static final int MULTIPLICATION = 2;
    private static final int ADDITION = 1;
    private static final int SUBTRACTION = 0;


    public Calculator() {
        this.gui = new CalculatorGui(this);
    }


    public String getAnswer(String text){
        try {

            if(isSingleCalc(text)) {
                return singleCalc(getFirstNum(text), getOperator(text), getSecondNum(text));
            }else {
                String[] nextAndIndexs = getNext(text);
                nextAndIndexs[0] = getAnswer(nextAndIndexs[0]);
                if(nextAndIndexs[0].equals("Invalid Calc")) {
                    return "Invalid Calc";
                }
                String step = insert(text,nextAndIndexs);
                return getAnswer(step);
            }
        } catch (Exception e) {
            return "Invalid Calc";
        }
    }

    private boolean isSingleCalc(String text) {
        boolean singleCalc = true;
        try {
            singleCalc(getFirstNum(text), getOperator(text), getSecondNum(text));
        } catch(Exception e) {
            singleCalc = false;
        }
        return singleCalc;
    }


    private String singleCalc(String firstNum, String operator, String secondNum) throws Exception {
        float ans = 0f;

        switch(operator) {
            case "+": ans = Float.parseFloat(firstNum) + Float.parseFloat(secondNum);
                break;
            case "-": ans = Float.parseFloat(firstNum) - Float.parseFloat(secondNum);
                break;
            case "*": ans = Float.parseFloat(firstNum) * Float.parseFloat(secondNum);
                break;
            case "/": ans = Float.parseFloat(firstNum) / Float.parseFloat(secondNum);
                break;
            default: throw new Exception();     
        }

        return "" + ans;
    }


    private String[] getNext(String text) {
        String currentHighestOper ="";  
        int beginingIndex,endIndex, index;
        beginingIndex = endIndex = index = 0;
        for(int i = 0; i < text.length(); i++) {
            if(isHigherPrecedence("" + text.charAt(i), currentHighestOper)) {
                currentHighestOper = "" + text.charAt(i);
                index = i;  
            }
        }

        if(text.charAt(index) != '(') {
            beginingIndex = getBeginingIndex(index, text);
            endIndex = getEndIndex(index, text);
        } else {
            beginingIndex = index + 1;
            endIndex = getEndBracketIndex(index, text);
        }

        String[] next = {text.substring(beginingIndex, endIndex),"" + beginingIndex,"" + endIndex, "" + text.charAt(index)};

        return next;
    }

    private int getBeginingIndex(int i, String text) {
        for(int j = i-1; j >= 0; j--) {
            if (isOperator("" +text.charAt(j)) && !isNegation(text,j)) {
                return j + 1;
            }
        }
        return 0;
    }


    private int getEndIndex(int i, String text) {
        for(int j = i + 1; j < text.length(); j++) {
            if (isOperator("" +text.charAt(j)) && !isNegation(text, j)) {
                return j;
            }       
        }
        return text.length();
    }


    private int getEndBracketIndex(int i, String text) {
        for(int j = i; j < text.length(); j++) {
            if(text.charAt(j) == ')') {
                return j;
            }
        }
        return text.length() - 1;
    }


    private boolean isOperator(String text) {
        if(text.charAt(0) == '+' ||
                text.charAt(0) == '-' ||
                text.charAt(0) == '*' ||
                text.charAt(0) == '/') {
            return true;
        } else {
            return false;
        }

    }


    private boolean isNegation(String text, int j) {
        if((j==0 || isOperator("" +text.charAt(j-1)) || isOperator("" +text.charAt(j-1))) && text.charAt(j) == '-') {
            return true;
        }else {
            return false;
        }
    }


    private boolean isHigherPrecedence(String oper, String highest) {
        int operValue = getPrecedenceValue(oper);
        int highestValue = getPrecedenceValue(highest);

        return (operValue >= highestValue)? true: false;
    }


    private int getPrecedenceValue(String oper) {
        switch(oper) {
            case "(": return BRACKETS;
            case "/": return DIVISION;
            case "*": return MULTIPLICATION;
            case "+": return ADDITION;
            case "-": return SUBTRACTION;
            default: return -1;
        }
    }

    private String insert(String mainAnswer,String[] nextAndIndexs) {
        StringBuilder sb = new StringBuilder(mainAnswer);

        if(!nextAndIndexs[3].equals("(")) {
            return sb.replace(Integer.parseInt(nextAndIndexs[1]),Integer.parseInt(nextAndIndexs[2]), nextAndIndexs[0]).toString();
        }else {
            return sb.replace(Integer.parseInt(nextAndIndexs[1]) - 1,Integer.parseInt(nextAndIndexs[2]) + 1, nextAndIndexs[0]).toString();
        }


    }


    private String getFirstNum(String text) {
        String firstNum = null;
        for(int i =0; i <text.length(); i++) {
            if(isOperator("" + text.charAt(i)) && !isNegation(text,i)) {
                firstNum = text.substring(0, i);
            }
        }
        return firstNum;
    }

    private String getOperator(String text) {
        String operator = null;
        for(int i =0; i <text.length(); i++) {
            if(isOperator("" + text.charAt(i))&& !isNegation(text,i)) {
                operator = text.substring(i, i+1);
            }
        }
        return operator;
    }

    private String getSecondNum(String text) {
        String secondNum = null;
        for(int i =0; i <text.length(); i++) {
            if(isOperator("" + text.charAt(i))&& !isNegation(text,i)) {
                secondNum = text.substring(i + 1, text.length());
            }
        }
        return secondNum;
    }

    public static void main(String[] args) {
        @SuppressWarnings("unused")
        Calculator calc = new Calculator();
    }

}

GUI:

public interface CalculatorGUIInterface {

    public void writeToScreen(String text);
    public void clearScreen();
    public String getScreenText();
    public String getAnswer();

}

public interface CalculatorInterface {
    public void writeToScreen(String text);
    public void clearScreen();
    public String getScreenText();
    public String getAnswer();
}

public interface CalculatorNumPanelInterface {
    public void writeToScreen(String text);
    public void clearScreen();
    public String getScreenText();  
}

public interface CalculatorOperPanelInterface {
    public void writeToScreen(String text);
    public void clearScreen();
    public String getScreenText();
    public String getAnswer();
}

import java.awt.Color;
import java.awt.ComponentOrientation;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.BoxLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.border.EmptyBorder;


@SuppressWarnings("serial")
public class CalculatorGui extends JFrame implements CalculatorGUIInterface {

    private class CalculatorPanel extends JPanel implements CalculatorInterface {

        private class NumberPanel extends JPanel implements CalculatorNumPanelInterface {

            private static final int NUMTOTAL = 10;

            private CalculatorPanel calcPanel;
            private JButton[] numButtons;

            public NumberPanel(CalculatorPanel calcPanel) {
                this.calcPanel = calcPanel;
                buildLayout();
                addButtons();
            }
            private void buildLayout() {
                this.setBorder(new EmptyBorder(0,0,0,5));
                this.setComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT);
                GridLayout layout = new GridLayout(4,3);
                layout.setHgap(1);
                layout.setVgap(1);

                this.setLayout(new GridLayout(4,3));

            }
            private void addButtons() {
                numButtons = new JButton[NUMTOTAL];

                for(int i = numButtons.length -1; i >= 0 ; i--) {
                    numButtons[i] = new JButton("" + i);
                    numButtons[i].setPreferredSize(new Dimension(60,40));
                    numButtons[i].setFont(new Font("Sans serif", Font.PLAIN, 18));
                    numButtons[i].addActionListener(
                            new ActionListener() {
                                @Override
                                public void actionPerformed(ActionEvent e) {
                                    String text = ((JButton)e.getSource()).getText().trim();
                                    if(getScreenText().equals("Invalid Calc")) {
                                        clearScreen();
                                        writeToScreen(text);
                                    }else {
                                        writeToScreen(text);
                                    }       
                                }
                            });
                    this.add(numButtons[i]);
                }
            }
            @Override
            public void writeToScreen(String text) {
                calcPanel.writeToScreen(text);
            }
            @Override
            public void clearScreen() {
                calcPanel.clearScreen();

            }
            @Override
            public String getScreenText() {
                return calcPanel.getScreenText();
            }

        }

        private class OperPanel extends JPanel implements CalculatorOperPanelInterface {

            private static final int ADD = 0;
            private static final int SUB = 1;
            private static final int MULT = 2;
            private static final int DIV = 3;
            private static final int OPENB = 4;
            private static final int CLOSEB = 5;
            private static final int CLEAR = 6;
            private static final int EQL = 7;


            private static final int OPERTOTAL = 8;

            private CalculatorPanel calcPanel;
            private JButton[] operButtons;

            public OperPanel(CalculatorPanel calcPanel) {
                this.calcPanel = calcPanel;
                buildLayout();
                addButtons();
            }           
            private void buildLayout() {
                GridLayout layout = new GridLayout(4,1);
                layout.setHgap(1);
                layout.setVgap(1);

                this.setLayout(new GridLayout(4,1));
            }           
            private void addButtons() {
                operButtons = new JButton[OPERTOTAL];

                operButtons[ADD] = makeButton(ADD, "+");
                operButtons[SUB] = makeButton(SUB, "-");
                operButtons[MULT] = makeButton(MULT, "*");
                operButtons[DIV] = makeButton(DIV, "/");
                operButtons[CLEAR] = makeButton(CLEAR, "CL");
                operButtons[EQL] = makeButton(EQL, "=");
                operButtons[OPENB] = makeButton(OPENB, "(");
                operButtons[CLOSEB] = makeButton(CLOSEB, ")");

                for(JButton button: operButtons) {
                    this.add(button);
                }   
            }           
            private JButton makeButton(int index, String label) {   

                operButtons[index] = new JButton(label);
                operButtons[index].addActionListener(
                        new ActionListener() {
                            @Override
                            public void actionPerformed(ActionEvent e) {
                                String screenText = "";
                                String text = ((JButton)e.getSource()).getText();
                                if(text.equals("=")) {
                                    screenText = getAnswer();
                                    clearScreen();
                                    writeToScreen(screenText);
                                }else if(text.equals("CL")) {
                                    clearScreen();  
                                }else {
                                    writeToScreen(text);
                                }
                            }       
                        });

                return operButtons[index];      
            }   
            @Override
            public String getScreenText() {
                return calcPanel.getScreenText();
            }
            @Override
            public void clearScreen() {
                calcPanel.clearScreen();

            }
            @Override
            public void writeToScreen(String text) {
                calcPanel.writeToScreen(text);

            }
            @Override
            public String getAnswer() {
                return calcPanel.getAnswer();
            }

        }


        private NumberPanel numPanel;
        private OperPanel operPanel;

        private CalculatorGui gui;

        public CalculatorPanel(CalculatorGui gui) {
            this.gui = gui;

            buildNumPanel();
            buildOperPanel();

            buildCalcPanel();


        }
        private void buildNumPanel() {
            this.numPanel = new NumberPanel(this);
        }

        private void buildOperPanel() {
            this.operPanel = new OperPanel(this);
        }

        private void buildCalcPanel() {
            this.setBorder(new EmptyBorder(10,0,0,0));
            this.setLayout(new BoxLayout(this, BoxLayout.X_AXIS));
            this.add(numPanel);
            this.add(operPanel);
        }


        @Override
        public void writeToScreen(String text) {
            gui.writeToScreen(text);    
        }

        @Override
        public String getScreenText() {
            return gui.getScreenText();
        }

        @Override
        public void clearScreen() {
            gui.clearScreen();
        }
        @Override
        public String getAnswer() {
            return gui.getAnswer();
        }
    }
    private Calculator calc;

    private JPanel mainPanel;// used to add padding around Calculator Panel

    private JTextField calcScreen;
    private CalculatorPanel calcPanel;

    public CalculatorGui(Calculator calc) {
        this.calc = calc;

        buildScreen();

        buildCalcPanel();

        buildMainPanel();

        buildCalculator();
    }



    private void buildScreen() {
        this.calcScreen = new JTextField();
        this.calcScreen.setEditable(false);
        this.calcScreen.setBackground(Color.WHITE);
        this.calcScreen.setPreferredSize(new Dimension(150,50));
        this.calcScreen.setHorizontalAlignment(JTextField.CENTER);
        this.calcScreen.setFont(new Font("Sans serif", Font.PLAIN, 30));

    }

    private void buildCalcPanel() {
        this.calcPanel = new CalculatorPanel(this);
    }

    private void buildMainPanel() {
        this.mainPanel = new JPanel();
        this.mainPanel.setBorder(new EmptyBorder(10,10,10,10));
        this.mainPanel.setLayout(new BoxLayout(this.mainPanel, BoxLayout.Y_AXIS));

        this.mainPanel.add(calcScreen);
        this.mainPanel.add(calcPanel);
    }

    private void buildCalculator() {

        this.add(mainPanel);
        this.setTitle("Calculator");
        this.pack();
        this.setResizable(false);
        this.setLocationRelativeTo(null);
        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        this.setVisible(true);
    }

    @Override
    public String getAnswer() {     
        return calc.getAnswer(calcScreen.getText());
    }

    @Override
    public void writeToScreen(String text) {
        calcScreen.setText(getScreenText() + text);     
    }

    @Override
    public void clearScreen() {
        calcScreen.setText(""); 
    }

    @Override
    public String getScreenText() {
        return calcScreen.getText();
    }
}

Welcome to CR! Although your code shows considerable effort from you, the crucial parts - as indicated by you - regarding validation and computing answers - are missing, and that makes this an off-topic question here. I'll suggest resolving those questions in Stackoverflow first, then swing by here with your complete solution. Cheers! — h.j.k., Jul 12 at 4:41
I agree that this code looks promising but isn't ready for review. You might want to take a look at some other calculator questions on this site for inspiration. — 200_success♦, Jul 12 at 5:23
Drop the method isValid as all does is finding out if getAnswer succeeds. This is a duplicate work, let getAnswer throw on invalid input (or return null or whatever). For now, forget parentheses and add some trivial evaluation code, even ignoring operator precedence should do. Find an operator, find the operands, execute. Repeat until done. Then you code can be reviewed and we can give you also tips for extensions (as a side result, don't ask for them). — maaartinus, Jul 12 at 14:44
I've tried to delete this thread countless times. I understand that I need to get the code working first before it can be reviewed. I'm working on it. — James, Jul 12 at 15:05
@James I don't think you have to delete this... once you are confident of filling up the unimplemented methods to the best of your ability, you can edit the question and then seek to reopen it. Like I said, this is almost on-topic already, so don't fret much. :) — h.j.k., Jul 12 at 16:07

SirPython · Answer 1 · 2015-07-21 02:27:56Z

I recommend creating an enum for your operations for three reasons:

The fields you are storing in the beginning (ADDITION, SUBTRACTION...) are structured just like an enum, because an enum orders it's items starting from 0.
You can assign the operator symbols to each operation
You can assign a method/function to each operator that is called when that operator is needed

Here is what the basic enum would look like, going off of the constants you created in the beginning of Calculator:

public enum Operator {
    ADDITION, SUBTRACTION, MULTIPLICATION, DIVISION
}

This, for example, sets the value Operator.MULTIPLICATION to 2.

This is a side note: looking back at your code, I see now why you had those constants: for the order in which they are executed.

Rather than creating an enum for these, I recommend creating an array (or something of that sort) and storing the values from Operator (maybe the int versions of the values) in there, along with the BRACKETS and ORDERS variables that you created.

Now, let's add to our enum the operator symbol that goes for each operation. For example, \ goes with the operator DIVISION.

Here is what the enum becomes:

public enum Operator {
    ADDITION('+'), SUBTRACTION('-'),
    MULTIPLICATION('*'), DIVISION('/');

    private final char symbol;
    private Operator(char symbol) {
        this.symbol = symbol;
    }
}

Alright, this was a big addition. Let's break it down:

What does ADDITION('+') mean?

When I wrote something like ADDITION('+'), this is basically calling the constructor that I made in the below of the enum and passing the character '+' to it.

This is all great, but now we should take it a step further: let's implement each operator's function as a lambda, and store this in the enum.

For this, we are going to need to create an interface that our lambdas will be from.

Since all the operator's functions take two parameters and return a single parameter, this makes our interface really easy:

public interface Operation {
    int solve(int a, int b);
}

This might be a little confusing. Keep reading for it to make sense.

Now that we have this interface for our lambdas, we can add our lambdas to the enum:

public enum Operator {
    ADDITION('+', (a, b) -> a + b),
    SUBTRACTION('-', (a, b) -> a - b),
    MULTIPLICATION('*', (a, b) -> a * b),
    DIVISION('/', (a, b) -> a / b);

    private final char symbol;
    private final Operation operation;

    private Operator(char symbol, Operation operation) {
        this.symbol = symbol;
        this.operation = operation;
    }
}

Let's look at a single lambda here: for example, the one for subtraction

(a, b) -> a - b

Does this look familiar? This is the same format as the solve function of the Operation interface: two integers are taken in, and one is returned.

This means that, provided that operation was a public field, we could subtract two parameters by calling

Operator.SUBTRACTION.operation.solve(int a, int b);

This will return a - b.

Our enum is looking really good. However, it is still not quite ready for use.

For starters, all the properties are private, so we are going to have to create getters for them. This is relatively easy:

public enum Operator {
    ...
    public char getSymbol() {
        return this.symbol;
    }
    public Operation getOperation() {
        return this.operation;
    }
}

Now we can use, for example

Operator.DIVISION.getSymbol()

And we will get the symbol for division which is '/'.

Now our enum is fully usable. However, it's not very helpful to the rest of the code.

Let's say the user hits the + button. Well, then what? How do we get the right enum so we can call the operation method?

We need to add some static functions to our Operator enum so that we can locate certain enum values using an input value, like the operator symbol. And, we can make this easier on ourselves by using the values method of an enum.

To create a method for finding an enum by it's operator symbol, we are going to have to do the following:

Get the operator symbol.
Loop through all the Operator values.
If the current Operator value's symbol is equal to the input symbol, return the current Operator symbol.

This comes out to this in Java:

public enum Operator {
    ...

    public static Operator getOperatorBySymbol(char symbol) {
        for(Operator operator : Operator.values()) {
            if(operator.getSymbol() == symbol) {
                return operator
            }
        }
    }
}

This should simplify the rest of your Java code by quite a lot.

Let's say the user enters some mathematical expression. After the calculator has decided the order of operations and all that other stuff, it can simply call a function like this:

public int compute(int a, int b, char operator) {
    return Operator.getOperatorBySymbol(operator).getOperation().solve(a, b);
}

Thanks to h.j.k. for this information:

Instead of creating a brand new interface for writing the lambda expression, you can simply use the IntBinaryOperator interface.

Well my mind is blown. To be honest I have never really used enums before, nor really seen much point in them, mainly because the way they were taught to me led me to believe they were equivalent to making a constant but with more code. This review you have given has completely changed my mind. Thank you for taking the time to look through what I wrote and I hope to hear more from you later :)! — James, Jul 15 at 1:46
@James Yes, in other languages, enums aren't really that special. But in Java, there are many, many things you can do with them. If you need, I can leave you a few resources about enums that I used. — SirPython, Jul 15 at 13:59
Usually I would just search them up on the docs.oracle.com website and use what information is on there to learn about them. If this isn't enough though feel free to send me more links :). — James, Jul 15 at 16:54
Very good answer, though I feel you can go a step further and have your enum implement IntBinaryOperator or DoubleBinaryOperator. ;) — h.j.k., Jul 21 at 1:08
@h.j.k. Wow! I haven't ever heard of those! Thanks so much for the information; I've edited my answer to incorporate your recommendation. If there is something I've left out, notify me. — SirPython, Jul 21 at 2:28

maaartinus · Answer 2 · 2015-07-16 00:45:47Z

Please, use your IDE to format the code.

Calculator

@SuppressWarnings("unused")
private CalculatorGui gui;

If it's unused, then it's useless. Just drop it:

public Calculator() {
    new CalculatorGui(this);
}

Simply creating the GUI without any assignment is fine. It doesn't get GC'd as it's registered internally somewhere in Swing.

But you've got this small part backwards. The GUI needs the engine, not the other way round. Getting rid of the GUI would allow you to write tests without any frames popping up.

public String getAnswer(String text){
    try {
        if(isSingleCalc(text)) {

Solving the simple case specially is quite often not needed. And often it's inefficient. You call singleCalc inside of isSingleCalc, throw away the result and call it again to get the result. A better pattern is something like

 try {
     return singleCalc(text);
 } catch (RuntimeException ignored) {
 }
 ... do it the hard way

If you want to avoid exception, then returning null can help

 String result = singleCalc(text);
 if (result != null) {
     return result;
 }
 ... do it the hard way

But usually just

 ... do it the hard way

is strictly better.

    } catch(Exception e) {

Don't catch Exception when something more specific suffices (RuntimeException).

Don't make your control flow that much exception driven. Often a simple test suffices.

    float ans = 0f;

Never use float unless

you have to save memory and it's in an array
you need it for speed and you know it really helps.

Float is damn imprecise and there's no reason for it here. Use double.

    switch(operator) {
        case "+": ans = Float.parseFloat(firstNum) + Float.parseFloat(secondNum);
            break;

You could use

double first = Double.parse(firstNum);
double second = Double.parse(secondNum);
    switch(operator) {
        case "+": return first + second;
        case "-": return first - second;

to gain some readability.

        default: throw new Exception();

This should be

        default: throw new IllegalArgumentException();

possible including an error message.

private String[] getNext(String text) {

This doesn't look like returning some "next". It seems to "split" or "parse" the string somehow, so call it accordingly.

    String currentHighestOper ="";

Use char where it suffices.

    int beginingIndex,endIndex, index;
    beginingIndex = endIndex = index = 0;

Don't predeclare your variables before you need them. Declare one variable per line like

    int beginingIndex = 0;
    int endIndex = 0;
    int index = 0;

and do not initialize variable which don't need it. That's Java, the compiler doesn't let you forget it.

The later part should look like

    int beginingIndex;
    int endIndex;
    if (text.charAt(index) != '(') {
        beginingIndex = getBeginingIndex(index, text);
        endIndex = getEndIndex(index, text);
    } else {
        beginingIndex = index + 1;
        endIndex = getEndBracketIndex(index, text);
    }

    String[] next = {text.substring(beginingIndex, endIndex),"" + beginingIndex,"" + endIndex, "" + text.charAt(index)};

That's perverted. You're using a String[] to pass integers. That's what classes are for:

 private static final class ParsedPart {
      String substring;
      int beginingIndex;
      int endIndex;
      char operator;
 }

private boolean isOperator(String text) {
    if(text.charAt(0) == '+' ||
            text.charAt(0) == '-' ||
            text.charAt(0) == '*' ||
            text.charAt(0) == '/') {
        return true;
    } else {
        return false;
    }

}

This could be written as

private boolean isOperator(String text) {
    return text.charAt(0) == '+' ||
            text.charAt(0) == '-' ||
            text.charAt(0) == '*' ||
            text.charAt(0) == '/');
}

but actually should be just

private boolean isOperator(char text) {
    return text == '+' ||
            text == '-' ||
            text == '*' ||
            text == '/');
}

unless you mean startsWithOperator.

private boolean isNegation(String text, int j) {
    if((j==0 || isOperator("" +text.charAt(j-1)) || isOperator("" +text.charAt(j-1))) && text.charAt(j) == '-') {
        return true;
    }else {
        return false;
    }
}

While j is a cool name for a loop variable, I'd avoid it here. The code is equivalent to

private boolean isNegation(String text, int index) {
    return (index==0 || isOperator(text.charAt(index-1)))
       && text.charAt(index) == '-';
}

as you repeated one term exactly.

In getFirstNum, getOperator, getSecondNum, you're doing about the same stuff. Actually, you should probably break after you find the operator. You need to get three things and it looks like you need three methods for it. But you don't. You could

write a trivial class aggregating firstNum, operator, and secondNum
or use a String[] (here, it'd alright)
or use a helper method like findFirstOperatorIndex
or (preferably) leave this simple case out and rely on the general processing

Summary

It's not bad, just a bit too complicated. I can still recall how hard is it to get one's first parser working. There are tools and there's theory for such stuff, though for a calculator an ad hoc solution is fine. Just make it simpler.

As you can see, I didn't cover everything as you've been given some feedback already. There's surely still a lot more to say. In case you want to work on this code further, I'd suggest to post the engine alone in a followup question (ideally with some JUnit tests).

"But usually just... do it the hard way" and "or (preferably) leave this simple case out and rely on the general processing" confused me slightly. Everything else I understand fully. Like making sure that I'm using the right variables for the right job and being more clear on what exceptions I'm dealing with. But the general idea behind the "getAnswer" method was to solve it recursively by first checking to see if it is in the smallest case, if it is not then do some manipulation to the string to step towards the smallest case. Surely removing the smallest case stops the solution? — James, Jul 16 at 1:59
@James Then you obviously can't remove it (I fully missed the recursion). But what you most probably can do, is using a simpler stopping condition: Instead of handling 1+2 specially, handle 3 specially. — maaartinus, Jul 16 at 2:05
That's a good idea. It would mean if I hit a single number in the calculator and hit equals it would just tell me the number instead of printing invalid calculation. It's just I'm too stupid to figure that one out, I guess I should break it down all the way check if its a single number, if it is return up and as I return up build the calculations and solve them as I'm building it up again? Anyway, I'll work on it. Thank you for your feedback :). I went for a big project in hopes I would get a lot of feedback. — James, Jul 16 at 2:23
@James tell me the number instead of printing invalid... and that's actually correct. +++ You can stop whenever there are no more operators. +++ I'm building it up again - actually, this dealing with strings makes it rather complicated in general. I'd split the string into parts like {"1", "+", "-", "22", "*", "-", "-", "3", so that each part is either an operator or a number. Once split, I'd never go back to a string (except for the final result). +++ You're welcome. It's a good project, but really split into parts. You can see that there's a lot of to say about a single thing, too. — maaartinus, Jul 16 at 2:52

asked	1 month ago
viewed	196 times
active	1 month ago

current community

your communities

more stack exchange communities

Java calculator using Swing

2 Answers 2

Calculator

Summary

Your Answer

Not the answer you're looking for? Browse other questions tagged java beginner swing calculator or ask your own question.

Linked

Hot Network Questions

current community

your communities

more stack exchange communities

Java calculator using Swing

2 Answers 2

Did you find this question interesting? Try our newsletter

Calculator

Summary

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged java beginner swing calculator or ask your own question.

Linked

Related

Hot Network Questions