Quadratic Equation Calculator with Java FX

I have designed a quadratic Equation Calculator with Java FX. It is divided into four separate classes:

• GUI.fxml to design the UI

• Logic class with the all the logic formulas and code that runs the program

• Main class which brings all the different components together

• Controller class

Any input is appreciated.

GUI.fxml

Logic.java

import org.apache.commons.math3.complex.Complex;
import org.apache.commons.math3.complex.ComplexFormat;

class Logic
{
    public static String function(double A,double B,double C)
    {
        ComplexFormat format = new ComplexFormat();

        double a = A;

        double b = B;

        double c = C;
        String answer = "Error";

        double x1 = 0 ,x2 = 0;
        x1 = (-b + (Math.sqrt((b*b) - (4*a*c))) )/(2*a);
        x2 = (-b - (Math.sqrt((b*b) - (4*a*c))) )/(2*a);
        if(Double.isNaN(x1))
        {
            String s1 = "Error";
            double r1 = (-b /(2.0*a));
            double i1 = (Math.sqrt(Math.abs((b*b) - (4*a*c))))/(2*a);
            Complex c1 = new Complex(r1,i1);
            s1 = format.format(c1);

            String s2 = "Error";
            double r2 = (-b /(2.0*a));
            double i2 = (Math.sqrt(Math.abs((b*b) - (4*a*c))))/(2*a);
            i2 = -i2;
            Complex c2 = new Complex(r2,i2);
            s2 = format.format(c2);
            answer = "Roots are  " + s1 + "   and   " + s2 + "  ";

        }
        else if(Double.isNaN(x1) == false && Double.isNaN(x2) == false)
        {
            answer = " The roots are "+Double.toString(x1) + " and " + Double.toString(x2) +"  ";

        }

        return answer;
    }   
}

Main.java

```
import javafx.application.Application;
import javafx.fxml.FXMLLoader;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.scen

-
The Logic.java class contains a lot code duplication. It's easy to get rid of it. For instance, the expression (bb) - (4a*c) is used four times. I would rather create a getDiscriminant method for it.

-
There are a few "strange" reassignments: for example, in the function method s1 and s2 are initially set to Error and unconditionally reassigned a few lines later. Creating them upon the assignment of the second value (the only one that is actually used) makes the code more clear.

-
The case when the equation has complex roots looks pretty convoluted in general. I suggest doing it this way:

if (Double.isNaN(x1)) {
        double real = -b / (2.0 * a);
        double imag = (Math.sqrt(Math.abs(getDiscriminant(a, b, c))) / (2.0 * a);
        Complex root1 = new Complex(real, imag);
        Complex root2 = new Complex(real, -imag);
        String s1 = format.format(root1);
        String s2 = format.format(root2);
        answer = "Roots are  " + s1 + "   and   " + s2 + "  ";
    }

It does the same thing as your code did, but no "unnecessary" variables are created.

-
Computing the roots and checking if they're Nan looks sort of confusing to me. I'd rather compute the discriminant and find complex roots if it's negative and real roots otherwise.

-
Double.isNaN(...) == false can be simplified to !Double.isNan(...). Comparing the values of boolean variable with constants is considered to be a bad practice.

-
I also do not the point of creating local variables are a, b and c. They are never modified, so I suggest using the method parameters directly.

-
The fact that the output looks slightly differently ("Roots are " + s1 + " and " + s2 + " " vs " The roots are "+Double.toString(x1) + " and " + Double.toString(x2) +" "), but has the same meaning seems strange. Is it deliberate? If yes, then it's fine. But I'd rather create a constant format string (for instance, it could be The roots are %s and %s) and use the String.format method to get rid of inconsistencies and make the code more readable.

StackExchange Code Review Q#146331, answer score: 2