0
\$\begingroup\$

I've just started with Java. The code below calculates Body Mass Index (BMI).

Could someone look for my code and assess it? Maybe I should write something more? All advice will be very helpful.

code:

import java.util.Scanner;

public class BMI {

    public static void main(String[] args) {

        Scanner scanner = new Scanner(System.in);

        double weight = 0;
        double height = 0;
        double bmi = 0;

        System.out.println("Please enter your details to calculate your Body Mass Index.");

        System.out.println("Enter your weight in kilograms:");
        weight = scanner.nextDouble();
        System.out.println("Enter your height in metres:");
        height = scanner.nextDouble();
        bmi = (weight / (height * height));
        System.out.println("Your BMI is: " + bmi);

        if (bmi >= 40) {
            System.out.println("Serious obesity");
        } else if (bmi >= 30) {
            System.out.println("Obesity");
        } else if (bmi >= 25) {
            System.out.println("Overweight");
        } else if (bmi >= 18) {
            System.out.println("Standard");
        } else {
            System.out.println("Underweight");
        }

        scanner.close();

    }

}
\$\endgroup\$
  • 1
    \$\begingroup\$ You definitely should write more. Please edit your question and tell us what this assesment is about? \$\endgroup\$ – t3chb0t Jun 29 '18 at 16:44
  • \$\begingroup\$ Welcome to Code Review! To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does \$\endgroup\$ – Malachi Jun 29 '18 at 16:58
  • \$\begingroup\$ while we know what BMI is and that your code calculates it, we would really like a brief description of what you did here to make this code unique from other BMI calculators \$\endgroup\$ – Malachi Jun 29 '18 at 17:00
  • 1
    \$\begingroup\$ Why on earth is this is being downvoted or closevoted? The only thing I can imagine is that people can't think of anything to say about it, and therefore assume there must be something wrong with the question. I'm sure if there were three or four glaring errors we'd have an answer and everyone would be happy. \$\endgroup\$ – Ben Aaronson Jun 29 '18 at 17:32
  • \$\begingroup\$ @BenAaronson well, the question looked quite different a while ago: here's my code, please review. Sometimes questions with errors (even if off-topic) contain more details about the code than this one at the beginning. \$\endgroup\$ – t3chb0t Jun 29 '18 at 18:14
4
\$\begingroup\$

Because this is such a simple program, there are really only a few basic things that can make it good or bad (beyond the fact that it actually works).

The good:

  • Formatting - this is all consistent, and matches the standard Java formatting
  • Naming - your variable names are all clear, concise and are good descriptions of the values they correspond to
  • User interaction - your messages are clear and have good spelling and grammar
  • Logic - There's no unnecessary complexity, and it's structured in a way that's clear to read and follow.

Could be improved:

  • Error handling - You take user input, but don't handle any cases where they provide something invalid. What if they give a negative or zero height, or write something that's not a number at all?
  • Structure - As you develop more complex programs, you'll move away from having all your code in the main method. As a starting point, even simple programs like this could benefit from being refactored using private, static methods. For example, the logic which takes a bmi and translates it into a descriptive word could be its own method, taking a double parameter and returning a String.

To call this "complete", you'd want to look at introducing more robust error-handling. However, as a beginning learner, you may not know exactly how to do this, and it might not necessarily be the next thing you want to learn. Whether or not you do that, I'd say you've demonstrated that you are capable of writing good code to achieve a simple goal using these basic features. I'd suggest that next you move onto more complex applications which will require things like collections, loops, and more complicated logic

\$\endgroup\$
  • \$\begingroup\$ Thank you so much for your comment. Your help makes me more determinated to learn coding. \$\endgroup\$ – Michal Jun 29 '18 at 18:27
3
\$\begingroup\$

Not bad for a beginner, and I agree with Ben Aaronson's comments. Maybe not now, but when you progress, you should change some things:

  • Break the code up in logical parts (here: methods), and every part should have one and only one task (Single Responsibility Principle). In your application, getting the data is one task, calculating is one task and displaying the results is one task
  • Error Handling (see Ben's answer)
  • Testing! Learn to write JUnit tests! It leads "automatically" to better code, not only because you avoid programming errors, but also because you learn to break up code to make it testable
  • Make your code both scalable, and applicable in different scenarios. For instance, the if-cascade is easy to write and to understand, but it doesn't scale (what if you need 20 cases?), and it is not freely applicable (e.g. you can't pass it around, the ifs are basically "glued" in place)

Although it might be a little bit too advanced for a beginner: Regarding the last point I want to show you a trick even some senior programmers don't know:

import java.util.Scanner;
import java.util.TreeMap;

public class BMI {

    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);
        System.out.println("Please enter your details to calculate your Body Mass Index.");
        System.out.println("Enter your weight in kilograms:");
        double weight = scanner.nextDouble();
        System.out.println("Enter your height in metres:");
        double height = scanner.nextDouble();
        scanner.close();
        double bmi = weight / (height * height);
        System.out.println("Your BMI is: " + bmi);

        TreeMap<Double, String> map = new TreeMap<>();
        map.put(40.0, "Serious obesity");
        map.put(30.0, "Obesity");
        map.put(25.0, "Overweight");
        map.put(18.0, "Standard");
        map.put(0.0, "Underweight");

        System.out.println(map.floorEntry(bmi).getValue());
    }
}

There is a data structure which can do all the if-checks for you, which makes the code trivial to write, scalable, and easy applicable: A TreeMap.

A Map is just a kind of dictionary where you can find "stuff" under a given key. In a TreeMap the keys are ordered. In Java's TreeMap you can not only ask for keys, but also for values between the keys, which get rounded up (ceiling...) or down (floor...) to the next key in the map.

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