3
\$\begingroup\$

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

  1. GUI.fxml to design the UI
  2. Logic class with the all the logic formulas and code that runs the program
  3. Main class which brings all the different components together
  4. Controller class

Any input is appreciated.

GUI.fxml

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.scene.control.Button?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.TextArea?>
<?import javafx.scene.control.TextField?>
<?import javafx.scene.image.Image?>
<?import javafx.scene.image.ImageView?>
<?import javafx.scene.layout.AnchorPane?>
<?import javafx.scene.text.Font?>

<AnchorPane prefHeight="372.0" prefWidth="405.0" xmlns="http://javafx.com/javafx/8.0.60" xmlns:fx="http://javafx.com/fxml/1" fx:controller="Controller">
   <children>
      <Button fx:id="clear" layoutX="174.0" layoutY="337.0" mnemonicParsing="false" onAction="#clear" text="Clear All" />
      <Label layoutX="12.0" layoutY="55.0" prefHeight="45.0" prefWidth="115.0" text="Enter a value for a : " />
      <TextField fx:id="text1" layoutX="116.0" layoutY="72.0" prefHeight="25.0" prefWidth="285.0" />
      <TextField fx:id="text2" layoutX="116.0" layoutY="120.0" prefHeight="25.0" prefWidth="285.0" />
      <TextField fx:id="text3" layoutX="116.0" layoutY="172.0" prefHeight="25.0" prefWidth="285.0" />
      <Label layoutX="10.0" layoutY="114.0" prefHeight="37.0" prefWidth="127.0" text="Enter a value for b : " />
      <Label layoutX="10.0" layoutY="169.0" prefHeight="31.0" prefWidth="115.0" text="Enter a value for c : " />
      <Label layoutX="9.0" layoutY="238.0" prefHeight="37.0" prefWidth="60.0" text="Answer : " />
      <Button fx:id="button" layoutX="171.0" layoutY="213.0" mnemonicParsing="false" onAction="#calculate" text="Calculate" />
      <Label layoutX="9.0" layoutY="6.0" prefHeight="37.0" prefWidth="391.0" text="                       Quadratic Equation Calculator">
         <font>
            <Font name="Cambria" size="18.0" />
         </font>
      </Label>
      <TextArea fx:id="result" layoutX="59.0" layoutY="244.0" prefHeight="84.0" prefWidth="343.0" wrapText="true" />
      <ImageView fx:id="equationImage" fitHeight="31.0" fitWidth="192.0" layoutX="116.0" layoutY="40.0" pickOnBounds="true" preserveRatio="true">
         <image>
            <Image url="@equation.png" />
         </image>


      </ImageView>
   </children>
</AnchorPane>

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.scene.image.Image;
import javafx.stage.Stage;

public class Main extends Application
{

    public static void main(String args[])
    {
        launch(args);
    }

    public void start(Stage stage)throws Exception
    {
        stage.setTitle("Quadratic Equation Calculator");

        stage.getIcons().add(new Image(getClass().getResourceAsStream("icon32.png"))); 

        Parent layout = FXMLLoader.load(getClass().getResource("GUI.fxml"));

        Scene scene = new Scene(layout);
        stage.setScene(scene);
        stage.show();
    }

}

Controller Class

import javafx.fxml.FXML;
import javafx.scene.control.TextArea;
import javafx.scene.control.TextField;

public class Controller
{
    @FXML 
    private TextField text1,text2,text3;
    @FXML private TextArea result;
    private String answer = "Error";
    public void calculate()
    {

        try
        {
        answer = Logic.function( Double.valueOf(text1.getText()),Double.valueOf(text2.getText()),Double.valueOf(text3.getText()) );
        //System.out.println(answer);
        }
        catch(Exception e)
        {

            answer = "Enter numerical values in all please";
        }
        result.setText(answer);
    }

    public void clear()
    {

        text1.setText("");
        text2.setText("");
        text3.setText("");
        result.setText("");
    }


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

  2. 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.

  3. 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.

  4. 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.

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

  6. 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.

  7. 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.

\$\endgroup\$
1
\$\begingroup\$

Single roots

Not all equations have two roots. For example,

A = 1
B = -2
C = 1

There is one root, 1.

This occurs when the discriminant (b*b - 4*a*c) is zero. And complex roots happen when the discriminant is negative.

Only work with ComplexFormat if necessary

        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) +"  ";

        }

Consider

        double denominator = 2.0 * A;
        double discriminant = B * B - 4.0 * A * C;

        if (discriminant > -.001 && discriminant < .001)
        {
            return " The root is " + Double.toString(-B / denominator) + "  ";
        }

        if (discriminant > 0.0)
        {
            double discriminantRoot = Math.sqrt(discriminant);
            double root1 = (-B + discriminantRoot) / denominator;
            double root2 = (-B - discriminantRoot) / denominator;

            return " The roots are " + Double.toString(root1) + " and "
                + Double.toString(root2) + "  ";
        }

        ComplexFormat format = new ComplexFormat();

Then calculate the complex roots. We know that they'll exist since the discriminant is negative.

This way, you only create format if needed. And only calculate denominator and discriminant once each.

I didn't see a reason to copy the parameters, so I didn't.

Since we return in the two simple cases, we can do away with the indent for the more complicated complex case.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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