I am creating a simple BMI Calculator to check if the user is overweight or not. I am not sure how much improvement I need but I do appreciate any comments as to whether the coding is inconsistent.

def introduction_page():
  print("Welcome to the BMI Calculator")
  to_start = str(input("Y to start. N to exit"))
  if to_start in ("Y","y"):
    print("We will calculate now")
    return main_page()

def main_page():
  found = False

  while not found:
    weight_store = float()
    height_store = float()
    user_weight = float(input("Please enter your weight(kg): "))
    weight_confirm =  str(input("Y to confirm. N to re-enter"))
    if weight_confirm in("y","Y"):
      weight_store = weight_store + user_weight
      while not found:
        user_height = float(input("Please enter your height(m): "))
        height_confirm = str(input("Y to confirm. N to re-enter"))
        if height_confirm in ("Y","y"):
          height_store = height_store + user_height
          total_height = (height_store * height_store)
          total_weight = (weight_store)
          BMI = (total_weight / total_height)
          print (int(BMI))
          if (BMI < 18.5 or BMI <25) :
            print("Normal Weight")
            found = True
          elif (BMI < 25 or BMI < 30):
            found = True
          elif (BMI < 30 or BMI < 40):
            found = True
            print("Morbid Obesity")
            found = True
          print("Please re-enter")
      print("Please re-enter!")


2 Answers 2




Per convention, you should indent your code with 4 spaces.


You should consider refactoring your code. This will allow the code to be more readable and will also allow you to reuse it. As Piotr mentioned it, the four functions can be extracted, which would essentially cover first two of his points.

Otherwise, the variable naming is good and easy to understand, following the PEP-8 guide. You can give the guide a read, to understand and follow a good style practice.

Variable input & logic

If you intend to assign a string to a variable using input, you don't need to specifically convert it to str. That is because input automatically assumes the variable to be a string.

Run the following snippet to see for yourself:

 new_variable = input('Enter some text: ')

Abundance of redundant variables & cluttered logic

Your code has too many variables that hold no real purpose. Your store and total variables are redundant, as you can work directly off your input.


height_store = height_store + user_height
total_height = (height_store * height_store)

You can re-write the above two lines using operators. They would look like this:

height_store += user_height
total_height = height_store ** 2

We won't use the first operator in this instance, as it won't be needed. We can narrow that block of logic down to the following code, which still works as intended.

user_weight = float(input('Please enter your weight(kg): '))
user_height = float(input('Please enter your height(m): '))
body_mass_index = (user_weight / user_height ** 2)
print(round(body_mass_index, 1))

BMI Result

The direction of comparison operators doesn't seem to be correct in your block of if statements. If statements don't require parentheses.

if 18.5 < body_mass_index < 25:
    print('Normal weight')
elif 25 < body_mass_index < 30:
elif body_mass_index > 30:

Also, you seemed to be missing a check for an underweight BMI.


Now that the logic is no longer cluttered, we can put it into different functions. Within those, we can restrain user input to certain values and handle errors. We will also some of the logic into a class.

As the tallest man ever was 2.72 meters tall, we are going to use that as the upper bound for _get_user_info.

class BodyMassIndex:

    def __init__(self, weight, height):
        self.weight = weight
        self.height = height

    def body_mass_index(self):
        return round(self.weight / self.height ** 2, 1)

    def score(self):
        if 18.5 < self.body_mass_index < 25:
            return 'normal weight'
        elif 25 < self.body_mass_index < 30:
            return 'overweight'
        elif self.body_mass_index > 30:
            return 'obese'
            return 'underweight'

    def print_score(self):
        print('Your Body Mass Index score is: {}'.format(self.body_mass_index))
        print('You are {}'.format(self.score))

def _get_user_info():
    while True:
            weight = float(input('Enter weight in kilograms: '))
            height = float(input('Enter height in meters: '))

            if 0 < weight and 0 < height < 2.72:
                return weight, height
                raise ValueError('Invalid height or weight')
        except ValueError:
            print('Invalid height or weight input')

def calculate_bmi():
    weight, height = _get_user_info()
    return BodyMassIndex(weight, height)

if __name__ == '__main__':
    bmi = calculate_bmi()

calculate_bmi functions gets user input and returns an instance of BodyMassIndex class, which performs the logic. If you want to handle weight and height input separately, you can split _get_user_info into two separate functions, _get_user_weight and _get_user_height.

Now, when we run the code, this is how it will behave:

Enter weight in kilograms: 78
Enter height in meters: 1.8
Your Body Mass Index score is: 24.1
You are normal weight

The code is perfectly reusable and you can use it as a module and import it in other programs.

>>> bmi = calculate_bmi()
Enter weight in kilograms: 78
Enter height in meters: 1.8
>>> bmi.height
>>> bmi.weight
>>> bmi.body_mass_index
>>> bmi.score
'normal weight'

for a starter I see tree aspects which you could upgrade:

  1. Improve code readability
  2. Make code reusable
  3. Improve validation / error handling.

From main_page() extract 4 functions:

  • get_user_height()
  • get_user_weight()
  • calculate_bmi(height, weight)
  • print_bmi_description(bmi)

In get_user_height() and get_user_weight() user input. What is min and max value which are correct in your model? e.g. nobody has weight of -1.0. Make sure that you can trust this functions i.e. they return only valid values.

calculate_bmi(height, weight) - extracting calculation code to distinct function makes it easy to reuse and test (unit tests or even simpler in interactive console).

print_bmi_description(bmi) - make sure your logic covers all possible values. By covers I mean it returns correct description or throws an error (ie. for negative BMIs).

  • You should remove redundant code eg. BMI < 18.5 or BMI <25 - check only for BMI <25.
  • Validate your logic. BMI < 30 or BMI < 40 seems incorrect. I think it should be BMI > 30 or BMI < 40 or in more pythonic way 30 < BMI < 40

I think it is enough for know. If you make this changes we could make next step :-)

  • \$\begingroup\$ Thank you for the comments! out of 5, please rate me. 1 (bad), 5(good) \$\endgroup\$
    – Anson Kho
    Jul 3, 2017 at 1:52
  • 2
    \$\begingroup\$ It's great you are looking for feedback. Forget about ratings, just make sure you deliver your best and make it better every time. According to Wikipedia article regarding BMI, your logic (ifs) is incorrect. Correctness is the most important property of good code - so make it work properly first. So for this time I would give you 1. Good luck with your journey! \$\endgroup\$ Jul 6, 2017 at 19:34
  • \$\begingroup\$ Thank you for your kind words! will definitely work hard!:) \$\endgroup\$
    – Anson Kho
    Jul 7, 2017 at 13:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.