Problem With Equations In JavaFX Calculator

Question

I have a problem with my JavaFX/Java calculator. When I enter an equation, it(the calculator) works fine, and the answer is correct, too. But when I press/key in another operation and another number after that, the result/sum/etc. will not be correct unless the operator I pressed/keyed in is the same one as before. Here's an example:

works: 2 + 6 = 8 + 2 = 10

doesn't work: 2 + 6 = 8 - 2 = 10

works: 6 x 2 = 12 x 2 = 26

doesn't work: 6 x 2 = 12 ÷ 2 = 26

I was wondering if there was any way I could fix this.

Here's my FXML Controller class:

package calculator;

import java.net.URL;
import java.util.ResourceBundle;
import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.fxml.FXMLLoader;
import javafx.fxml.Initializable;
import javafx.scene.Node;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.input.KeyCode;
import javafx.stage.Stage;
import java.lang.Math;

public class Calculator implements Initializable {

    double data = 0d;
    int operation = -1;
    private boolean start = false;

    @FXML
    private Label display;

    @FXML
    private Button two;

    @FXML
    private Button five;

    @FXML
    private Button four;

    @FXML
    private Button three;

    @FXML
    private Button one;

    @FXML
    private Button six;

    @FXML
    private Button seven;

    @FXML
    private Button multi;

    @FXML
    private Button add;

    @FXML
    private Button divide;

    @FXML
    private Button minus;

    @FXML
    private Button equals;

    @FXML
    private Button clear;

    @FXML
    private Button zero;

    @FXML
    private Button nine;

    @FXML
    private Button eight;

    @FXML
    void handleButtonAction(ActionEvent event) {

        if(start)
        {
            display.setText("");
            start = false;
        }

        if(event.getSource() == one)
        {
            display.setText(display.getText() + "1");
        }
        else if(event.getSource() == two)
        {
            display.setText(display.getText() + "2");
        }
        else if(event.getSource() == three)
        {
            display.setText(display.getText() + "3");
        }
        else if(event.getSource() == four)
        {
            display.setText(display.getText() + "4");
        }
        else if(event.getSource() == five)
        {
            display.setText(display.getText() + "5");
        }
        else if(event.getSource() == six)
        {
            display.setText(display.getText() + "6");
        }
        else if(event.getSource() == seven)
        {
            display.setText(display.getText() + "7");
        }
        else if(event.getSource() == eight)
        {
            display.setText(display.getText() + "8");
        }
        else if(event.getSource() == nine)
        {
            display.setText(display.getText() + "9");
        }
        else if(event.getSource() == zero)
        {
            display.setText(display.getText() + "0");
        }
        else if(event.getSource() == clear)
        {
            display.setText("");
        } 
        else if(event.getSource() == add)
        {
            data = Float.parseFloat(display.getText());
            operation = 1;
            display.setText("");
        }
        else if(event.getSource() == minus)
        {
            data = Float.parseFloat(display.getText());
            operation = 2;
            display.setText("");
        }
        else if(event.getSource() == multi)
        {
            data = Float.parseFloat(display.getText());
            operation = 3;
            display.setText("");
        }
        else if(event.getSource() == divide)
        {
            data = Float.parseFloat(display.getText());
            operation = 4;
            display.setText("");
        }
        else if(event.getSource() == equals)
        {
            Float secondOperand = Float.parseFloat(display.getText());
            switch(operation)
            {
                case 1: //Addition
                    Double ans = data + secondOperand;
                    display.setText(String.valueOf(ans));
                    //data = ans;
                    break;
                case 2: //Subtraction
                    ans = data - secondOperand;
                    display.setText(String.valueOf(ans));
                    //data = ans;
                    break;
                case 3: //Multiplication
                    ans = data * secondOperand;
                    display.setText(String.valueOf(ans));
                    //data = ans;
                    break;
                case 4: //Division
                    ans = 0d;
                    try{
                    ans = data / secondOperand;
                    }catch(Exception ex){display.setText("Error");}
                    display.setText(String.valueOf(ans));
                    //data = ans;
                    break;
            }
            if(event.getSource() != divide && event.getSource() != add && event.getSource() != multi && event.getSource() != minus)
            {
                start = true;
            }
        }

    }

@FXML
    private void send2sceneconver(ActionEvent event) throws Exception{
        Parent rootBMI = FXMLLoader.load(getClass().getResource("ConversionCal.fxml"));

        Scene scene2 = new Scene(rootBMI);
        Stage calS = (Stage) ((Node) event.getSource()).getScene().getWindow();

        calS.setScene(scene2);
        calS.show();
    }

    @Override
    public void initialize(URL url, ResourceBundle rb) {
        // TODO
    }  

}

Any help would be appreciated.

At a glance: you need to initialize `operation` after the `switch` statement. Also you may want to read [what is a debugger](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems). For more help please post [mre]. — c0der, Jul 08 '19 at 04:45
`//data = ans;` should be uncommented too. And the handling using `start` is complicated. — Joop Eggen, Jul 08 '19 at 13:26
@JoopEggen you don't need to uncomment because if you do another operation on the answer it gets set in this line `data = Float.parseFloat(display.getText());` which is the first thing any of the operations do — Matt, Jul 08 '19 at 13:30

score 3 · Answer 1 · answered Jul 08 '19 at 13:55

You do the following for the press of the equals button

else if(event.getSource() == equals)
{
    ...
    if(event.getSource() != divide && event.getSource() != add && event.getSource() != multi && event.getSource() != minus)
    {
        start = true;
    }
}

Since source is equals and none of the others, you always set start to true which on the press of the next button replaces the content of display with the empty string:

if(start)
{
    display.setText("");
    start = false;
}

This results in a NumberFormatException being thrown by the next call to

data = Float.parseFloat(display.getText());

Since the assignment to the operation field happens after this call, pressing an operator button just after pressing equals does not result in the update of the field's value and the old operator is always kept.

What you should do instead is introduce a field telling you, if the first operand is available or has to be parsed.

private final Set<Button> numbers = new HashSet<>();
private final Map<Button, Integer> operators = new HashMap<>();

@Override
public void initialize(URL url, ResourceBundle rb) {
    numbers.addAll(Arrays.asList(zero, one, two, three, four, five, six, seven, eight, nine));
    operators.put(add, 1);
    operators.put(minus, 2);
    operators.put(multi, 3);
    operators.put(divide, 4);
}

private double data;
private boolean dataAvailable = false;

@FXML
private void handleButtonAction(ActionEvent event) {
    Button source = (Button) event.getSource();

    if (source == clear) {
        dataAvailable = false;
        display.setText("");
        operation = 0;
    } else if (source == equals) {
        double secondOperand;
        try {
            secondOperand = Double.parseDouble(display.getText());
        } catch (NumberFormatException ex) {
            return; // only continue, if parsing is successful
        }
        double result;
        switch (operation) {
            case 1: //Addition
                result = data + secondOperand;
                break;
            case 2: //Subtraction
                result = data - secondOperand;
                break;
            case 3: //Multiplication
                result = data * secondOperand;
                break;
            case 4: //Division
                double res = data / secondOperand;
                if (Double.isFinite(res)) {
                    result = res;
                } else {
                    // TODO: division by zero
                }
                break;
            default:
                return; // ignore press, if operand is not set yet
        }
        display.setText(Double.toString(result));
        operation = 0;
        data = result;
        dataAvailable = true;
    } else if (numbers.contains(source)) {
        if (!dataAvailable) { // just ignore input, if = gave us the first operand
            display.setText(display.getText() + source.getText());
        }
    } else {
        Integer op = operators.get(source);
        if (op != null) {
            if (!dataAvailable) {
                try {
                    data = Double.parseDouble(display.getText());
                } catch (NumberFormatException ex) {
                    return; // do nothing on invalid input
                }
            } else {
                dataAvailable = false;
            }
            display.setText("");
            operation = op;
        }
    }
}

score 2 · Accepted Answer · answered Jul 08 '19 at 13:17

So I'm not exactly sure why its there but the problem is with this if statment

    if(start)
    {
        display.setText("");
        start = false;
    }

I have tested it with your works vs doesn't work equations and all seems to be fine. The reason that was breaking it was it was reseting the display as it was doing the next operation so it looks fine when watching it but the data gets cleared as it trying to do the operation. so it had no last data to pull from. I commented it out and the rest seemed to work here is my class for verification

import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.fxml.FXMLLoader;
import javafx.fxml.Initializable;
import javafx.scene.Node;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.stage.Stage;

import java.net.URL;
import java.util.ResourceBundle;


public class Calculator implements Initializable {

    double data = 0d;
    int operation = -1;
    private boolean start = false;

    @FXML
    private Label display;

    @FXML
    private Button two;

    @FXML
    private Button five;

    @FXML
    private Button four;

    @FXML
    private Button three;

    @FXML
    private Button one;

    @FXML
    private Button six;

    @FXML
    private Button seven;

    @FXML
    private Button multi;

    @FXML
    private Button add;

    @FXML
    private Button divide;

    @FXML
    private Button minus;

    @FXML
    private Button equals;

    @FXML
    private Button clear;

    @FXML
    private Button zero;

    @FXML
    private Button nine;

    @FXML
    private Button eight;

    @FXML
    void handleButtonAction(ActionEvent event) {

//        if(start)
//        {
//            display.setText("");
//            start = false;
//        }

        if(event.getSource() == one)
        {
            display.setText(display.getText() + "1");
        }
        else if(event.getSource() == two)
        {
            display.setText(display.getText() + "2");
        }
        else if(event.getSource() == three)
        {
            display.setText(display.getText() + "3");
        }
        else if(event.getSource() == four)
        {
            display.setText(display.getText() + "4");
        }
        else if(event.getSource() == five)
        {
            display.setText(display.getText() + "5");
        }
        else if(event.getSource() == six)
        {
            display.setText(display.getText() + "6");
        }
        else if(event.getSource() == seven)
        {
            display.setText(display.getText() + "7");
        }
        else if(event.getSource() == eight)
        {
            display.setText(display.getText() + "8");
        }
        else if(event.getSource() == nine)
        {
            display.setText(display.getText() + "9");
        }
        else if(event.getSource() == zero)
        {
            display.setText(display.getText() + "0");
        }
        else if(event.getSource() == clear)
        {
            display.setText("");
        }
        else if(event.getSource() == add)
        {
            data = Float.parseFloat(display.getText());
            operation = 1;
            display.setText("");
        }
        else if(event.getSource() == minus)
        {
            data = Float.parseFloat(display.getText());
            operation = 2;
            display.setText("");
        }
        else if(event.getSource() == multi)
        {
            data = Float.parseFloat(display.getText());
            operation = 3;
            display.setText("");
        }
        else if(event.getSource() == divide)
        {
            data = Float.parseFloat(display.getText());
            operation = 4;
            display.setText("");
        }
        else if(event.getSource() == equals)
        {
            Float secondOperand = Float.parseFloat(display.getText());
            switch(operation)
            {
                case 1: //Addition
                    Double ans = data + secondOperand;
                    display.setText(String.valueOf(ans));
                    //data = ans;
                    break;
                case 2: //Subtraction
                    ans = data - secondOperand;
                    display.setText(String.valueOf(ans));
                    //data = ans;
                    break;
                case 3: //Multiplication
                    ans = data * secondOperand;
                    display.setText(String.valueOf(ans));
                    //data = ans;
                    break;
                case 4: //Division
                    ans = 0d;
                    try{
                        ans = data / secondOperand;
                    }catch(Exception ex){display.setText("Error");}
                    display.setText(String.valueOf(ans));
                    //data = ans;
                    break;
            }
//            if(event.getSource() != divide && event.getSource() != add && event.getSource() != multi && event.getSource() != minus)
//            {
//                start = true;
//            }
        }
    }

    @FXML
    private void send2sceneconver(ActionEvent event) throws Exception{
        Parent rootBMI = FXMLLoader.load(getClass().getResource("ConversionCal.fxml"));

        Scene scene2 = new Scene(rootBMI);
        Stage calS = (Stage) ((Node) event.getSource()).getScene().getWindow();

        calS.setScene(scene2);
        calS.show();
    }

    @Override
    public void initialize(URL url, ResourceBundle rb) {
        // TODO
    }
}

Also side note you can remove the implements Initializable if your not using it

score 0 · Answer 3 · answered Jul 14 '19 at 15:45

I also figured out another way, to declare the [operation =] before the [data =], like this:

        else if(event.getSource() == divide)
        {
            operation = -1;
            operation = 4;
            data = Float.parseFloat(display.getText());
            display.setText("");
            dot.setDisable(false);
        }

Problem With Equations In JavaFX Calculator

3 Answers3