Quadratic Expression Calculator 2

Question

After taking some great advice on this question, I'm hoping for some more feedback on my quadratic expression calculator.

This is my code as stands:

public class Quadratic_Equations {

    private final 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 \"exit\" at any time to exit");
            double a = promptUserUntilValidInput("Please enter a:");
            double b = promptUserUntilValidInput("Please enter b:");
            double c = promptUserUntilValidInput("Please enter c:");
            System.out.println(formatOutput(calculateQuadraticFormula(a, b, c)));
            System.out.println();
        }
    }

    private static String formatOutput(Set<Double> resultsFromEquation) {
        if (resultsFromEquation == null)
            return "These numbers do not compute - they produce an illegal result.";
        else {
            return resultsFromEquation.toString();
        }
    }

    private static double promptUserUntilValidInput(String prompt){
        if (prompt.equals("exit")) {
            sc.close();
            System.exit(0);
        }
        boolean valid = false;
        Double value = new Double(0.0);
        while(!valid){
            System.out.println(prompt);
            value = tryParse(sc.nextLine());
            if (value != null)
                valid = true;
        }
        return value;
    }

    private static Double tryParse(String text){
        try{
            return Double.parseDouble(text);
        }
        catch (NumberFormatException e){
            return null;
        }
    }

    public static Set<Double> calculateQuadraticFormula(double a, double b, double c) {
        Set<Double> results = new HashSet<Double>();
        double temp = (b * b) - (4 * a * c);
        if (temp < 0)
            return null;
        if( temp == 0 ) {
            results.add(-b / 2 * a);} 
        else if( temp > 0) {
            results.add( (-b + Math.sqrt(temp) ) / (2 * a) );
            results.add( (-b - Math.sqrt(temp) ) / (2 * a) );
        }
        return results;
    }
}

Before I go on, there are a few things I should point out.

I'm aware this could be better if the quadratic stuff was in a class of its own - however, for the purposes of this simple exercise I'm happy skipping that.

My main issues are:

1. static Scanner. Some people on previous question suggested making this a private variable, but then it needs to get passed to everything. Is this worth it?

2. Does it look okay to close the scanner (and exit the program) where I do?

3. Do other methods look single-responsibility-ish enough?

Answer 1

The static Scanner is a problem, a better way to do it is with the Java try-with-resources. If you add the scanner as an input parameter to the prompt function, you can do:

Change your prompt method from:

private static double promptUserUntilValidInput(String prompt){

to

private static double promptUserUntilValidInput(Scanner sc, String prompt){

and then change your main-method to:

public static void main(String[] args) {
    try (Scanner sc = new Scanner(System.in)) {
        while (true) {
            System.out.println("Please enter a, b and c or enter \"exit\" at any time to exit");
            double a = promptUserUntilValidInput(sc, "Please enter a:");
            double b = promptUserUntilValidInput(sc, "Please enter b:");
            double c = promptUserUntilValidInput(sc, "Please enter c:");
            System.out.println(formatOutput(calculateQuadraticFormula(a, b, c)));
            System.out.println();
        }
    }
}

Instead of doing the System.exit, I would prefer a less abrupt exit. Consider a custom Exception, like "EndOfProgramException" which you can catch....

private static class EndOfProgramException extends RuntimeException {
    EndOfProgramException () {
        super();
    }
}

and throw this exception in your prompt method:

    if (prompt.equals("exit")) {
        throw new EndOfProgramException();
    }

and catch it in the main:

public static void main(String[] args) {
    try (Scanner sc = new Scanner(System.in)) {
        while (true) {
            System.out.println("Please enter a, b and c or enter \"exit\" at any time to exit");
            double a = promptUserUntilValidInput(sc, "Please enter a:");
            double b = promptUserUntilValidInput(sc, "Please enter b:");
            double c = promptUserUntilValidInput(sc, "Please enter c:");
            System.out.println(formatOutput(calculateQuadraticFormula(a, b, c)));
            System.out.println();
        }
    } catch (EndOfProgramException e) {
        System.out.prinln("Exiting...");
    }
}

As for the single-responsibility, it would look pretty clean with the above changes.

Answer 2

Bugs

• calculateQuadraticFormula() calculates wrong results for double roots. Watch your operator associativity!

  if( temp == 0 ) {
      results.add(-b / 2 * a);}
  

• The "exit" feature doesn't work. The prompt would never be "exit", would it?

  private static double promptUserUntilValidInput(String prompt){
      if (prompt.equals("exit")) {
          sc.close();
          System.exit(0);
      }
  

Design issues

• Class names should be nouns, and the name should reflect their purpose. I recommend QuadraticSolver as the class name.
• The calculateQuadraticFormula() function could then be simply named solve().
• The class should not hold a Scanner as a member variable, as that would violate the Single Responsibility Principle. It can be a local variable in main() that gets passed to the prompting function. (The scanner should be created using a try-with-resources block.)
• Returning null from the solver is annoying for the caller to handle. I suggest returning an empty set if there are no real roots.
• I suggest returning two copies of the result if it is a double root, but that is a matter for debate.

Style issues

• You assign the expression (b * b) - (4 * a * c) to a variable named temp. Why not use a descriptive name discriminant?

• The promptUserUntilValidInput() function could be better if you removed its tryParse() helper, I think. (Furthermore, promptUser… is redundant — who else are you going to prompt?)

  Also, since there is a System.exit() hidden inside, it needs to be documented. (Actually, @rolfl's suggestion to throw an exception is better.)

  /**
   * Prompts the user to enter a number, and retries until the input is a
   * valid double.  Calls System.exit(0) if the input is "exit", or if EOF
   * is encountered.
   */
  private static double promptUntilValidInput(String prompt, Scanner sc) {
      do {
          try {
              System.out.print(prompt);
              String input = sc.nextLine();
              if ("exit".equals(input)) {
                  System.exit(0);
              }
              return Double.parseDouble(input);
          } catch (NoSuchElementException eof) {
              System.out.println();
              System.exit(0);
          } catch (NumberFormatException retryOnBadInput) {
          }
      } while (true);
  }