HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Quadratic expression calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
expressionquadraticcalculator

Problem

I'm hoping to get some feedback on how to improve and optimise a program I wrote which takes three values as input (via the terminal) and uses them in the quadratic formula.

I'm aware this is mostly micro-optimisation, but any advice whatsoever would be much appreciated.

```
public class Quadratic_Equations {
static Scanner sc = new Scanner (System.in);

public static void main(String[] args) {
while (true) {
System.out.println("Please enter a, b and c or enter -1 at any time to exit");
double a = tryParse(sc.nextLine());
a = checkIfValidNumber(a);
if (a == -1)
break;
double b = tryParse(sc.nextLine());
b = checkIfValidNumber(b);
if (b == -1)
break;
double c = tryParse(sc.nextLine());
c = checkIfValidNumber(c);
if (c == -1)
break;

System.out.println(useQuadraticFormula(a, b, c));
System.out.println();
}
sc.close();
}

private static double checkIfValidNumber(double number) {
while (number == -1.0) {
System.out.println("You didn't enter a valid number for a. Please try again.");
number = tryParse(sc.nextLine());
}
return number;
}

public static Double tryParse(String text) {
try {
return new Double(text);
} catch (NumberFormatException e) {
return -1.0;
}
}

public static String useQuadraticFormula(double a, double b, double c) {
double result1;
double result2;
if (Math.pow(b, 2) - (4 a c) <= 0)
return "These numbers do not compute - they produce an illegal result.";
result1 = (-b + Math.sqrt(Math.pow(b, 2) - (4 a c))) / (2 * a);
result2 = (-b - Math.sqrt(Math.pow(b, 2) - (4 a c))) / (2 * a);
return String.valueOf(result1) + ", " + String.valueOf(result2)

Solution

A few things jump me here:

  1. Class Names



Your class name is queer. The Java standard is to use PascalCasing for class names. You use Pascal_Snake_Case. That's kinda funny ;) Also you named your class after what it contains somewhere deep down, and not what it's responsible for. I'd instead probably name it Program. As it's nothing else, than that. I'd expect a class named Quadratic_Equations to be an enum containing different Equations.

  1. checkIfValidNumber



I like the name. It's good, it could describe what it does. I don't like the print-statement and that you have a loop in there. This is one of these "side-effects". Your function name makes me expect something different.
I expect a function applying some constraint to a number and returning true or false depending on the validity of that number to the constraint.

You also hide your whole process behind that. Also: why should a user not enter -1.0 for your quadratic equation? It makes no sense to use a number within the valid range to check for validity. instead use boolean return type in your tryParse and make use of the out-parameter.

  1. Reading user input.



double a = tryParse(sc.nextLine());
  a = checkValidNumber(a);


This makes no sense. You should also move the first tryParse(sc.nextLine()); into a method.

Instead I would expect:

double a = promptUserUntilValidInput("Please enter a:");


You can move the whole reading to this method.

private double promptUserUntilValidInput(String prompt){
    boolean valid = false;
    double value = 0.0;
    while(!valid){
       System.out.println(prompt);
       //this is the c# attempt...
       valid = tryParse(sc.nextLine(), out value);

       //You would need to do something like this in java
       Double result = tryParse(sc.nextLine());
       valid = result != null;
       value = (double)result;
    }
    return value;
}


tryParse would then (in C#) return true, when parsing the number was successful. You'd just need to assign to an out parameter before returning in the method:

private boolean tryParse(String text, out double val){
     try{
        val = new Double(text);
        return true;
     }
     catch (NumberFormatException e){
        return false;
     }
}


This way you also eliminate all checks for -1.0.

But as this is java we need to make it a little more complicated. I'm here heavily relying on @Uri Agassi's answer

private Double tryParse(String text){
    try{
       Double value = new Double(text);
       return value;
    }
    catch (NumberFormatException e){
       if(text.matches("/^quit$/gi"))
           Application.Exit;
       else
         return null;
     }
}

Code Snippets

double a = tryParse(sc.nextLine());
  a = checkValidNumber(a);
double a = promptUserUntilValidInput("Please enter a:");
private double promptUserUntilValidInput(String prompt){
    boolean valid = false;
    double value = 0.0;
    while(!valid){
       System.out.println(prompt);
       //this is the c# attempt...
       valid = tryParse(sc.nextLine(), out value);

       //You would need to do something like this in java
       Double result = tryParse(sc.nextLine());
       valid = result != null;
       value = (double)result;
    }
    return value;
}
private boolean tryParse(String text, out double val){
     try{
        val = new Double(text);
        return true;
     }
     catch (NumberFormatException e){
        return false;
     }
}
private Double tryParse(String text){
    try{
       Double value = new Double(text);
       return value;
    }
    catch (NumberFormatException e){
       if(text.matches("/^quit$/gi"))
           Application.Exit;
       else
         return null;
     }
}

Context

StackExchange Code Review Q#46293, answer score: 13

Revisions (0)

No revisions yet.