Simple BMI calculator in Java

Question

A similar question must have been asked a thousand times, but for the life of me I can't figure this out. I am new to Java and I want to create a programme that asks a user for his or her height and weight, and calculate the corresponding BMI. I want to throw exceptions for the wrong input (has to be a double, non-negative and not zero) and I want to catch any mathematical errors that may occur. The follow code does that, but I have exact duplication for handling the input of height and weight. How can I simplify this into a more concise format without losing any functionality?

import java.util.InputMismatchException;
import java.util.Scanner;

public class BMIcalculator {
    public static void main(String[] args){
        double BMI = calculateBMI();
        System.out.print("\nYour BMI is " + BMI + ".");
    }

    public static double calculateBMI() {
        // Scanner allows us to request a users input
        Scanner scan = new Scanner(System.in);
        // Initialize doubles as zero values
        double w=0, h=0;

        // Ask for user input (weight)
        System.out.println("Please enter your weight");
        while(true) {
            try {
                w = scan.nextDouble();
                if (w <= 0) throw new ArithmeticException();
                break;
            } catch (InputMismatchException ime) {
                System.err.println("Ahw, error: " + ime.getMessage());
                continue;
            } catch (ArithmeticException ae) {
                System.err.println("Weight can't be less than or equal to zero!");
                continue;
            }
        }
        // Ask for user input (height)
        System.out.println("Please enter your height");
        while(true) {
            try {
                h = scan.nextDouble();
                if (h <= 0) throw new ArithmeticException();
                break;
            } catch (InputMismatchException ime) {
                System.err.println("Ahw, error: " + ime.getMessage());
                continue;
            } catch (ArithmeticException ae) {
                System.err.println("Height can't be less than or equal to zero!");
                continue;
            }
        }
        // try-catch not necessary; we only get here when our input is valid


        // we have to close the input connection to the user
        scan.close();
        return w / (h * h);
    }
}

Answer 1

Avoiding code repetition

As pointed out in some of the other answers, getting a double value from the keyboard can be abstracted into its own method to avoid code repetition:

private static double getDouble(Scanner scanner, String metric) {
    System.out.println("Enter " + metric);
    double result;
    while (!scanner.hasNextDouble() || (result = scanner.nextDouble()) <= 0) {
        System.out.println(metric + " must be a positive number.");
        scanner.next();
    }
    return result;
}

try-with-resources

If you are on Java 7 and above, you should use try-with-resources for efficient handling of the underlying I/O resource:

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        double weight = getDouble(scanner, "Weight (kg)");
        double height = getDouble(scanner, "Height (m)");
        System.out.println("Your BMI is: " + /* calculation */);
    }
}

Units

It's important to note that BMI is expressed in units of \$\frac{kg}{m^2}\$, so you should definitely display the units of measurement too. Otherwise, you'll need extra methods to perform the conversion to kilograms and meters. :)

Comments

Simply put, comments should explain the why, not how. Most of your comments now are just descriptive, i.e. on the how, which is already apparent in the code. Comments are sometimes seen as redundant as they are prone to being outdated. Therefore, comment sparingly, and only when you really need it to explain something that is non-obvious. Even so, try to make the code 'obvious' first. :)

Answer 2

I'm not a java programmer so this might not be right.

The weight and height code are almost the same so could you try:

public static double getANumber (String szThing)
{
    double result = 0.0;
    // Ask for user input
    System.out.println("Please enter your " + szThing);
    while(true) {
        try {
            result = scan.nextDouble();
            if (result <= 0) throw new ArithmeticException();
            break;
        } catch (InputMismatchException ime) {
            System.err.println("Ahw, error: " + ime.getMessage());
            continue;
        } catch (ArithmeticException ae) {
            System.err.println(szThing + " can't be less than or equal to zero!");
            continue;
        }
    }

Answer 3

Change the while loops to this. Check the scanner to see if their is the correct input ready -> if not tell them its wrong and skip to the next line. If it is a number, check it to make sure its > 0. If its <= 0 then we set the w back to 0 so the loop tries again.

while (w == 0) {
    if(scan.hasNextDouble()){
        w = scan.nextDouble();
        if(w <= 0){
            System.err.println("Weight can't be less than or equal to zero!");
            scan.nextLine();
            w = 0;
         }
    }else {
         System.err.println("Please enter a number!");
         scan.nextLine();
    }
}

Using exceptions for this is not the best way to find errors, unless you have been told to do that way, this is a better solution (I think?)

import java.util.Scanner;

public class BMIcalculator {

    public static void main(String[] args) {
        BMIcalculator bmi = new BMIcalculator();
        double BMI = bmi.calculateBMI();
        System.out.print("\nYour BMI is " + BMI + ".");
    }

    public double calculateBMI() {
        // Scanner allows us to request a users input
        Scanner scan = new Scanner(System.in);
        // Initialize doubles as zero values
        double w = 0, h = 0;

        // Ask for user input (weight)
        System.out.println("Please enter your weight");
        while (w == 0) {
            if(scan.hasNextDouble()){
                w = scan.nextDouble();
                if(w <= 0){
                    System.err.println("Weight can't be less than or equal to zero!");
                    scan.nextLine();
                    w = 0;
                }
            }else {
                System.err.println("Please enter a number!");
                scan.nextLine();
            }
        }
        // Ask for user input (height)
        System.out.println("Please enter your height");
        while (h == 0) {
            if(scan.hasNextDouble()){
                h = scan.nextDouble();
                if(h <= 0){
                    System.err.println("Height can't be less than or equal to zero!");
                    scan.nextLine();
                    h = 0;
                }
            }else {
                System.err.println("Please enter a number!");
                scan.nextLine();
            }
        }
        // try-catch not necessary; we only get here when our input is valid

        // we have to close the input connection to the user
        scan.close();
        return w / (h * h);
    }
}

Answer 4

Having the main method within your logic class is never good. Create something like an Application class and keep the main method there! :)

Then, notice you have 3 steps, which are :

1. Ask for weight
2. Ask for height
3. Compute

I feel like this could use at least two methods. ReadWeight and ReadHeight. Now, what does this look like :

public class Application {
    public static void main(String[] args){
        double BMI = BMIcalculator.calculate();
        System.out.print("\nYour BMI is " + BMI + ".");
    }
}

public class BMIcalculator {

    public static double calculate() {
        Scanner scan = new Scanner(System.in);
        double weight= readWeight(scan);
        double height = readHeight(scan);

        scan.close();
        return weight / (height * height);
    }

    private static double readWeight(Scanner scanner) {
        System.out.println("Please enter your weight");
        while(true) {
            try {
                double weight = scan.nextDouble();
                if (weight <= 0) throw new ArithmeticException();
                return weight;
            } catch (InputMismatchException ime) {
                System.err.println("Ahw, error: " + ime.getMessage());
                continue;
            } catch (ArithmeticException ae) {
                System.err.println("Weight can't be less than or equal to zero!");
                continue;
            }
        }
    }

    private static double readHeight(Scanner scanner) {
        System.out.println("Please enter your height");
        while(true) {
            try {
                double height = scan.nextDouble();
                if (height <= 0) throw new ArithmeticException();
                return height;
            } catch (InputMismatchException ime) {
                System.err.println("Ahw, error: " + ime.getMessage());
                continue;
            } catch (ArithmeticException ae) {
                System.err.println("Height can't be less than or equal to zero!");
                continue;
            }
        }
    }
}

Notice I've changed h and w for height and weight. It's clearer!

One could argue that readHeight and readWeight should be in the same method, and have slightly different parameters to decide which String to show, but I think it's best to keep them separated. After all, maybe you'll end up asking for a feet/foot height as "5'1''" instead of inches. That way your methods can change their logic without impacting the other one.