How does this quadratic formula program look?

I'm new to Java and this is my first program. I'm trying to verify that my thought processes are good.

Let me know of any improvements infor the code, whether it's too few/too many comments, poorly structured code, etc.

import javax.swing.*;
import java.math.*;
import java.util.*;

class Root
{
public static void main(String[] args)
{
//Variable & Constant Declaration
double coeffA=0.0; //Variable for Coefficient A
double coeffB=0.0; //Variable for Coefficient B
double coeffC=0.0; //Variable for Coefficient C
double xroot1=0.0; //Variable for the first root
double xroot2=0.0; //Variable for the second root
double discriminant=0.0; //Varaible for the discriminant
double root1complex=0.0; //Variable for part a of the complex root
double root2complex=0.0; //Variable for part b of the complex root
boolean contloop=true; //Variable for continuing loop
String loop="y"; //Variable for input choice of continuing loop

Scanner sc = new Scanner(System.in); //An object to read from the keyboard

try{
do{

if (loop.equals("y")){

//Input
System.out.println("Please enter the value for the first coefficient: "); //Asks the user to input the value for the A term
coeffA=sc.nextDouble(); //Captures the keyboard input and stores it to the variable coeffA

System.out.println("Please enter the value for the second coefficient: "); //Asks the user to input the value for the B term
coeffB=sc.nextDouble(); //Captures the keyboard input and stores it to the variable coeffB

System.out.println("Please enter the value for the third coefficient: "); //Asks the user to input the value for the C term
coeffC=sc.nextDouble(); //Captures the keyboard input and stores it to the variable coeffC

//Calculations
discriminant=coeffB*coeffB-(4*coeffA*coeffC); //Calculates the discriminant
xroot1=(-coeffB+(Math.sqrt(discriminant)))/(2*coeffA); //Calculates the first root of the polynomial
xroot2=(-coeffB-(Math.sqrt(discriminant)))/(2*coeffA); //Calculates the second root of the polynomial

//Output
System.out.println("The Equation is:"); //Displays the value for Coefficient C
System.out.println(coeffA+"x^2 + "+coeffB+"x + "+ coeffC); //Displays the equation to the user
System.out.println("The discriminant is: " + discriminant); //Displays the discriminant to the user

if(discriminant<0){
root1complex=-coeffB/(2*coeffA); //Calculates the first part of the complex root
root2complex=Math.sqrt(-discriminant)/(2*coeffA); //Calculates the second part of the complex root
System.out.println("The Equation has Imaginary roots."); //Displays to the user that there are imaginary roots
System.out.println("The Roots are: " + root1complex + "+" + root2complex + "i" + " and " + root1complex + "-" + root2complex + "i.");
if (coeffA==0){
System.out.println("*NaN stands for \"Not a Number.\"*");
} //Ends If
} //Ends If

else if(discriminant==0){
System.out.println("The Root is: " + xroot1 + "."); //Displays the root
if (coeffA==0){
System.out.println("*NaN stands for \"Not a Number.\"*");
} //Ends If
} //Ends If

else if (discriminant>0){
System.out.println("The Roots are: " + xroot1 + " and " + xroot2 +"."); //Displays the roots
if (coeffA==0){
System.out.println("*NaN stands for \"Not a Number.\"*");
} //Ends If
} //Ends If

System.out.println("Do you wish to perform another calculation?"); //Asks the user if they would like to perform another calculation
loop=sc.nextLine(); //Captures the next string input and stores it to the variable loop
} //Ends If

else if (loop.equals("n")){
System.out.println("Quit."); //Prints "Quit"
contloop=false; // Sets variable contloop to false thereby allowing therby allowing the program to exit the loop and finish
} //Ends If

else if ((!loop.equals("y")) || (!loop.equals("n"))){
System.out.println("Please enter \"y\" or \"n\"."); //Requests the user to enter a valid input
loop=sc.nextLine(); //Captures the next string input and stores it to the variable loop
} //Ends If
}while(contloop==true); //Ends Loop
System.exit(0); //Exits Java
} //End Try

catch (InputMismatchException e){
System.out.println("Invalid input format\nOnly numbers (decimals or intergers) are valid input formats.\nPlease restart program.");
} //End Catch
} //End Main
} //End Class


migrated from stackoverflow.comFeb 29 '12 at 21:54

This question came from our site for professional and enthusiast programmers.

• looks spaghettic. Not reusable. Merging interactive IO with the calculation (business logic). – user unknown Feb 29 '12 at 22:10
• Consider using BigDecimal instead of double. – Clockwork-Muse Mar 2 '12 at 18:14

I agree to everything, Matthew Flaschen wrote, but like to add some more things:

You don't declare and initialize your variables with dummy values upfront. Instead, you restrict the scope as much as possible, and delay the initialization, if possible, to first usage.

public static void main (String [] args)
{
boolean contloop = true;
String loop = "y";

Scanner sc = new Scanner (System.in);

try {
do {
if (loop.equals ("y")) {

System.out.println ("Please enter the value for the first coefficient: ");
double coeffA = sc.nextDouble ();

System.out.println ("Please enter the value for the second coefficient: ");
double coeffB = sc.nextDouble ();

System.out.println ("Please enter the value for the third coefficient: ");
double coeffC = sc.nextDouble ();

double discriminant = coeffB * coeffB - (4 * coeffA * coeffC);
double xroot1 = (-coeffB + (Math.sqrt (discriminant))) / (2 * coeffA);
double xroot2 = (-coeffB - (Math.sqrt (discriminant))) / (2 * coeffA);


This makes reasoning about what the variable is used for, who is influenced by modifying it and refactoring much more easy.

I would suggest much more white space between the tokens. It's better readable for the human eye. The machine will not mention it. Compare yourself:

xroot1 = (-coeffB + (Math.sqrt (discriminant))) / (2 * coeffA);
xroot1=(-coeffB+(Math.sqrt(discriminant)))/(2*coeffA);


If you use comments, put them infront of what is commented, so that javadoc maps them correctly.

Avoid System.exit(0); //Exits Java! Your code can't be called from another Java program, if you use it, because, as written in your comment, it doesn't just stop your program, but the whole JVM. But you will often see it in tutorials.

In your case, a return; is all you need.

Trying to refactor your code, so that we would have a main loop with 3 steps:

loop
calculation
format output
while user ok


with 3 method calls, and a class for the values, I mentioned, that the control is surprisingly complicated for the task, which is only whether there should be another loop or not. But we have a variable loop, another one contloop. Can they be out of sync? Why do we have two?

But the main aspect is, to split the task into 3 smaller parts. Think of the possibility to reuse the calculation in a webinterface, Where you don't use System.out.

• Thanks! Like I stated, it's my first time. In most examples I had seen on the web the variables were initialized at the point of usage. My teacher for some has continually shown us up front and wants our code formatted like that when printed. – Caleb Feb 29 '12 at 23:01
• Also: Restrict your try ... catch to only the code that can actually throw the execeptions. – Martin Schröder Mar 1 '12 at 0:20
• If you want to use Javadoc, you also need to use Javadoc comments. This means specific tags within /** ... */. – Matthew Flaschen Mar 1 '12 at 5:51
• @Eagle: I can't discuss with your teacher in this thread. He is welcome to visit this site and argue his viewpoint, but we can't discuss his authority in this question without him being here. Ask your teacher why he wants the variables up front. – user unknown Mar 1 '12 at 8:00
• @user unknown: I think it's just a preference for assignments on her part. And unless I get a really poor reason from her it's not worth talking about. I don't want to appear to disrespect her even on a website she'll never be on. I was in a rush in my last comment because the fire alarm went off as I was typing, so I couldn't make myself clear. :) – Caleb Mar 7 '12 at 17:56

//Variable for Coefficient A (or equivalent for any other variable)

//Variable & Constant Declaration

//An object to read from the keyboard (clear from standard library docs)

You also don't need the end comments:

//Ends Loop


for the same reason. It's an essential part of the language syntax, and many editors let you toggle between the start and end brace.

You have some small repetition, which you can factor out to a variable or method, like the part about NaN.

A more realistic comment is something like:

// Quadratic formula from Optimized Math Algorithms p. 46


This is silly here, since the quadratic formula is well known. But it would be useful for more novel algorithms.

Also, don't repeat calculations like Math.sqrt(discriminant) or 2*coeffA. Use an intermediate variable.

I would also separate the input, calculation, and output phases into different methods.

• Hey, I'm really very new to this. I pretty much made this off of googling questions and reading the ide. I'm fuzzy in terminology. Could you give me an example of how I would separate the input, calculation, etc. into different methods? Do you mean something like this: public output(){ } – Caleb Mar 7 '12 at 17:33
• @Caleb, yes, more or less. You'll need the output to take the values to print as arguments. The input method should return the user's input. – Matthew Flaschen Mar 7 '12 at 19:38

Here is my take:

import java.util.Scanner;

class Root {
private static final Scanner sc = new Scanner(System.in);

public static double askForCoefficient(String ordinal) {
while (true) {
try {
System.out.println("Please enter the value for the " + ordinal + " coefficient: ");
String s = sc.nextLine();
return Double.parseDouble(s);
} catch (NumberFormatException ex) {
System.out.println("Invalid input. Please enter a number (decimal or interger)");
}
}
}

double a = 0.0;
while (a == 0.0) {
if (a == 0.0) {
System.out.println("The first coefficient can't be 0 in a quadratic polynom.");
}
}
return new double[]{a, b, c};
}

public static boolean yesOrNo(String answer) {
}

String loop = "";
System.out.println("Do you wish to perform another calculation (y/n)?");
while (!yesOrNo(loop)) {
loop = sc.nextLine();
if (!yesOrNo(loop)) {
}
}
return loop.equalsIgnoreCase("y");
}

public static Complex[] calculate(double a, double b, double c) {
double discriminant = b * b - (4 * a * c);
System.out.println("The discriminant is " + discriminant);
Complex root1 = new Complex(-b, 0).plus(Complex.sqrt(discriminant)).multReal(0.5 / a);
Complex root2 = new Complex(-b, 0).minus(Complex.sqrt(discriminant)).multReal(0.5 / a);
return (root1.equals(root2)) ? new Complex[]{root1} : new Complex[]{root1, root2};
}

public static void main(String[] args) {
do {
System.out.println("The Equation is:");
System.out.println(coeffs[0] + "x^2 + " + coeffs[1] + "x + " + coeffs[2]);

Complex[] roots = calculate(coeffs[0], coeffs[1], coeffs[2]);

if (roots.length == 1) {
System.out.println("The Root is " + roots[0] + ".");
} else {
if (!roots[0].isReal()) {
System.out.println("The Equation has imaginary roots.");
}
System.out.println("The Roots are: " + roots[0] + " and " + roots[1]);
}
System.out.println("Quit.");
}

public static class Complex {
public final double real;
public final double imag;

public Complex(double real, double imag) {
this.real = real;
this.imag = imag;
}

public Complex plus(Complex that) {
return new Complex(this.real + that.real, this.imag + that.imag);
}

public Complex minus(Complex that) {
return new Complex(this.real - that.real, this.imag - that.imag);
}

public boolean isReal() {
return imag == 0.0;
}

public Complex multReal(double d) {
return new Complex(d * real, d * imag);
}

public static Complex sqrt(double d) {
return d >= 0.0
? new Complex(Math.sqrt(d), 0.0)
: new Complex(0.0, Math.sqrt(-d));
}

public String toString() {
if (imag == 0.0) return "" + real;
else if (imag > 0) return real + "+" + imag + "i";
else return real + "" + imag + "i";
}

public boolean equals(Object o) {
Complex that = (Complex) o;
return this.real == that.real && this.imag == that.imag;
}

}

}


I broke the class in several methods with clear, testable sub-tasks. Most notably things got cleaner and easier when I used the right data type, which is here Complex. Don't stick with the standard classes / primitives if they don't really fit. Of course if you want to reuse and extend Complex, you should make it a top level class.

The last condition statement is definitely wrong:

else if ((!loop.equals("y")) || (!loop.equals("n")))


It should be &&, not ||.

Why choose a do while conditional statement? If you just wanna make sure it will be run at least one time, use a while loop.

The main function are is large; split it to small function. You will maybe find that some code can be reused.

For a comment like this:

Scanner sc = new Scanner(System.in); //An object to read from the keyboard


Everyone knows what this code does, so no need to comment. You comment simple code which already explains itself, then you repeat yourself. Remember the 'DRY' rule; don't let comments repeat yourself.

I don't quite understand the algorithmic part. Maybe you can add one MathUtil class to help you do the algorithm.

For a beginning programming class it doesn't matter, but a good principle is to separate the part of your code that do calculation from the parts that do input and output. The "Calculations" block should be in a separate method.

• Thanks! I'll definitely try to do that for my next program. Things kinda blended together when I got going... :D Pretty much I googled things for answers because my teacher, like so many people's, melts my mind. Plus, she doesn't teach fast enough for my tastes. I was homeschooled so I mainly look up how to do things and learn from examples before she teaches it. So my code is going to look poor for a little while. And beginning or not, it should be perfect. Because, well it just should be. :) – Caleb Mar 7 '12 at 17:43