Simple calculator app made in Android Studio (Java)

Question

I'm a college student in my first year of my bachelor in IT. I recently decided to try some app development with Android Studio and Java. For my first project, I made a basic calculator app.

Here you can see how it looks:

The calculator app:

• can take multiple digits and floating point numbers as input
• has operator precendence
• can add, substract, divide and mutiply

My main idea was to have an ArrayList in wich every number and every operator gets stored.

Then for the calculation, I searched the ArrayList for the operator and took the numbers that are to left and to the right of it, and replaced those with the answer of those.

Here is the code of my mainActivity.java:

package com.example.calculatormk2;

public class MainActivity extends AppCompatActivity {

    private ArrayList input = new ArrayList();
    private StringBuilder number = new StringBuilder();
    private boolean calculationDone = false;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
    }

    public void onNumberClick(View v) {
        number.append(getBtnTxt(v));
        updateView(v);
    }

    public void onOperatorClick(View v) {
        addNumberToInput(number);
        addOperatorToInput(v);
        updateView(v);
    }

    public void onEqualsClick(View v) {
        addNumberToInput(number);
        calculate(input);
        clearInput();
    }

    public void calculate(ArrayList i) {
        while (i.contains("x") || i.contains("÷")) {
            int indexOperatorTimes = i.indexOf("x");
            int indexOperatorDivideBy = i.indexOf("÷");
            int indexOperatorMain;

            if (indexOperatorTimes < indexOperatorDivideBy && indexOperatorTimes != -1) {
                indexOperatorMain = indexOperatorTimes;
            } else {
                indexOperatorMain = indexOperatorDivideBy;
            }
            if (indexOperatorDivideBy == -1) {
                indexOperatorMain = indexOperatorTimes;
            }

            double firstNumber;
            double secondNumber;
            String operator = i.get(indexOperatorMain).toString();
            double answer;

            if (i.get(indexOperatorMain - 1) instanceof Integer) {
                firstNumber = (int) i.get(indexOperatorMain - 1);
            } else {
                firstNumber = (double) i.get(indexOperatorMain - 1);
            }
            if (i.get(indexOperatorMain + 1) instanceof Integer) {
                secondNumber = (int) i.get(indexOperatorMain + 1);
            } else {
                secondNumber = (double) i.get(indexOperatorMain + 1);
            }

            if (operator.equals("x")) {
                answer = firstNumber * secondNumber;
            } else {
                answer = firstNumber / secondNumber;
            }

            i.remove(indexOperatorMain - 1);
            i.remove(indexOperatorMain - 1);
            i.remove(indexOperatorMain - 1);
            i.add(indexOperatorMain - 1, answer);
            System.out.println(i);
        }

        while (i.contains("+") || i.contains("-")) {
            int indexOperator1 = i.indexOf("+");
            int indexOperator2 = i.indexOf("-");
            int indexOperatorDef;

            if (indexOperator1 < indexOperator2 && indexOperator1 != -1) {
                indexOperatorDef = indexOperator1;
            } else {
                indexOperatorDef = indexOperator2;
            }
            if (indexOperator2 == -1) {
                indexOperatorDef = indexOperator1;
            }

            double firstNumber;
            double secondNumber;
            String operator = i.get(indexOperatorDef).toString();
            double answer;

            if (i.get(indexOperatorDef - 1) instanceof Integer) {
                firstNumber = (int) i.get(indexOperatorDef - 1);
            } else {
                firstNumber = (double) i.get(indexOperatorDef - 1);
            }
            if (i.get(indexOperatorDef + 1) instanceof Integer) {
                secondNumber = (int) i.get(indexOperatorDef + 1);
            } else {
                secondNumber = (double) i.get(indexOperatorDef + 1);
            }

            if (operator.equals("+")) {
                answer = firstNumber + secondNumber;
            } else {
                answer = firstNumber - secondNumber;
            }
            i.remove(indexOperatorDef - 1);
            i.remove(indexOperatorDef - 1);
            i.remove(indexOperatorDef - 1);
            i.add(indexOperatorDef - 1, answer);
        }
        calculationDone = true;
        TextView t = findViewById(R.id.mainOutput);
        String s = i.get(0).toString();
        t.setText(s);
    }

    public void addOperatorToInput(View v) {
        input.add(getBtnTxt(v));
    }

    public void addNumberToInput(StringBuilder s) {
        if (s.toString().contains(".")) {
            input.add(Double.parseDouble(s.toString()));
        } else {
            input.add(Integer.parseInt(s.toString()));
        }
        clearNumber();
    }

    public String getBtnTxt(View v) {
        return ((Button) v).getText().toString();
    }

    public void clearNumber() {
        number.delete(0, number.length());
    }

    public void clearInput() {
        input.clear();
    }

    @SuppressLint("SetTextI18n")
    public void updateView(View v) {
        TextView mainOutput = findViewById(R.id.mainOutput);
        if (calculationDone) {
            mainOutput.setText("");
            calculationDone = false;
        }
        mainOutput.setText(mainOutput.getText() + getBtnTxt(v));
    }
}

In total it took me about 10 hours to make this and I'm pretty happy with it (because I didn't expect to get it to even work) even though there is a lot of double code. There is also a lot of code which only exists to cast the numbers to the right datatype. I realised late when working on the app that the get() function of an ArrayList returns the item as an object and it gave a lot of trouble retrieving something out of it. Looking back, I probably should have done this differently but I went into this project without a proper plan and just winged it.

I really would appreciate some feedback. You can be honest about it, even if it's garbage.

If, by any chance, anyone wants to take a look at the full project. Here is the GitHub page: https://github.com/PhilipNousPXL/Calculator-mk2.git

Answer 1

nice to see some android code here - thanks for sharing

violation of Single Responsibility Principle (SRP)

you MainActivity is responsible to build up the GUI and is also responsible for being a calculator! That is too much responsibility for your Activity.

Solution

create a Class Calculator and that is responsible for taking the input and returning a result.

violation of Integration Operation Segregation Principle (IOSP)

this is the reason why your calculate method is so messy (sorry, no offense, really!)

IOSP calls for a clear separation:

• Either a method contains exclusively logic, meaning transformations, control structures or API invocations. Then it’s called an Operation.
• Or a method does not contain any logic but exclusively calls other methods within its code basis. Then it’s called Integration.

Solution

create a integration method that handles the logical program flow, create several operation methods that do the bit shuffling!

example for an integration method (this is just a method stub):

public void calculate(ArrayList i) {
    handleMultiplier(i);
    handleAdditions(i);
}

example for an operation method (this is just an example, not real code!!!):

public double getLeftOperand(...){
    if (i.get(indexOperatorDef - 1) instanceof Integer) {
        return (int) i.get(indexOperatorDef - 1);
    } else {
        return (double) i.get(indexOperatorDef - 1);
    }
}

uncecessarily complex code

it is overkill to parse the arguments to either double or int Keep It Simple Stupid... keep all your input in String and just parse ONLY to double (see getLeftOperand() up above)!

pseudo concurrency

You make your Button unclickable while the calculator is doing some cpu-intensive Task. Either make a real Async-Task (see Handler.post(Runnable)) and make some real separation from your your GUI-Thread.

Or just leave it be and skip the enabling/disabling (that's fine for me)

Open Question

what happens, if you push + twice? that case is not handled properly, you cannot even remove your accidently pushed input. (yes - that also counts for the other arithmetic operations)

Naming

well, thats just me being picky, but one-letter-variables (i, v, etc) are no more used today since very IDE provides code completion!

Android Activity Life Cycle

have a look at the Android Activity Life Cycle and take a thought on what happens to the input, if your Activity is unplanned closed? Your Input will be wiped. But you have left open the requirements for that, so feel free to just look a bit deeper into android and take that point for further projects :-)

Answer 2

Your approach for expression evaluation is pretty clever. However, take a look at how the GNU Coreutils "expr" program processes mathematical expressions recursively. The recursive approach allows you to support parenthesis, functions and unary operators quite easily. It's in C but it should be fairly easy to understand. After that, extract your expression evaluation into a standalone class so that you can unit test it efficiently without being distracted by the UI.

Having done the same app previously myself, I found that the hardest part was tokenizing the input string into operators and operands (for example, expr skips this completely and just requires the user to provide a space seprated string :)). You should extract that logic into a separate class that takes a String as an input and returns a List where every element is either a complete number or an operand. Again, once it's a separate class, you can test it wihout being distracted by the expression evaluation and other unrelated things.

Now you have a pipeline that consists of the app that reads the input, the tokenizer that converts it to a list and the evaluator that calculates the result. Every component does only one thing.

Don't use shorthand like getBtnTxt(View v) in method names. It's a bit confusing as it works on Views instead of Buttons. getTextFrom(View v) would communicate more clearly what the method does.