14
\$\begingroup\$

I am a beginner in the python language. I wrote a BMI calculator and I am wondering if there are any improvements I could make to the code. This is the code below:

print('Enter your weight in kilo:')
weightInKilo = float(input())

print('Enter your height in feet:')

heightInFeet = float(input())

heightInMeter = heightInFeet * 12 * 0.025

bodyMassIndex = weightInKilo / (heightInMeter ** 2)

if bodyMassIndex < 15:
    print('Your BMI = ' + str(bodyMassIndex)  + ' You are very severely underweight.')

elif bodyMassIndex >= 15 and bodyMassIndex <= 16 :
    print('Your BMI = ' + str(bodyMassIndex)     + ' You are severely underweight.')

elif bodyMassIndex > 16 and bodyMassIndex <= 18.5:
    print('Your BMI = ' + str(bodyMassIndex) + ' You are underweight.')

elif bodyMassIndex > 18.5 and bodyMassIndex <= 25:
    print('Your BMI = ' + str(bodyMassIndex) + ' You are Normal(healthy weight).')

elif bodyMassIndex > 25 and bodyMassIndex <= 30:
    print('Your BMI = ' + str(bodyMassIndex) + ' You are overweight.')


elif bodyMassIndex > 30 and bodyMassIndex <= 35:
    print('Your BMI = ' + str(bodyMassIndex) + ' You are moderately obese.')

elif bodyMassIndex > 35 and bodyMassIndex <= 40:
    print('Your BMI = ' + str(bodyMassIndex) + ' You are severely obese.')

elif bodyMassIndex > 40:
    print('Your BMI = ' + str(bodyMassIndex) + ' You are very severely obese.')

input('Please press Enter to exit')
\$\endgroup\$
  • 1
    \$\begingroup\$ FYI, some 2017 research indicates that waist-to-height ratio is a better obesity index when predicting 'metabolic syndrome' for individuals. You could extend your code to return that information as well, or at least note in the output that the code judges an 'obese' condition based only on BMI, out of multiple possible obesity indices. \$\endgroup\$ – user117529 Dec 31 '18 at 23:50
41
\$\begingroup\$

This kind of programming exercice, despite its apparent simplicity, is a good opportunity to learn various things.

Style

Python has a Code Style guide called PEP 8. If you begin with Python, I highly recommend reading it every now and then and trying to apply it.

In your case, a few things could be improved regarding style:

  • blank lines between the different branches of if are more an inconvenience from my point of view
  • variables should follow the snake_case naming convention (instead of camelCase)

Builtin input

As you've noticed for the end of the function, the input builtin takes an optional prompt parameter. This could be used at the beginning of the function as well.

Duplicated code

A lot of code looks like the same line with minimal variations. Having duplicated code makes the code more tedious to read and harder to maintain (if you need to change something, you'll need to change it in many places). One of the principles of software programming is Don't Repeat Yourself (also written DRY).

In your case, we could store the body category in a variable and only print the ouput from a single place:

Chained comparison

This is very specific to Python but instead of body_mass_index > 30 and body_mass_index <= 35, we can write: 30 < body_mass_index <= 35 using chained comparisons.

Magic numbers

Magic Numbers are usually frowned upon. In our case, one needs a bit of thinking to understand where the 12 and 0.025 come from. A better way to handle this is to store them in a variable with a meaningful name.

I am not quite sure about the best naming convention for names related to conversion constant. I've followed the names suggested by Solomon Ucko in the comments.


At this stage, we have the following code:

INCHES_PER_FOOT = 12
METERS_PER_INCH = 0.0254

weight_in_kilo = float(input('Enter your weight in kilo:'))
height_in_feet = float(input('Enter your height in feet:'))

height_in_meter = height_in_feet * INCHES_PER_FOOT * METERS_PER_INCH
body_mass_index = weight_in_kilo / (height_in_meter ** 2)

if body_mass_index < 15:
    category = 'very severely underweight'
elif 15 <= body_mass_index <= 16 :
    category = 'severely underweight'
elif 16 < body_mass_index <= 18.5:
    category = 'underweight'
elif 18.5 < body_mass_index <= 25:
    category = 'Normal(healthy weight)'
elif 25 < body_mass_index <= 30:
    category = 'overweight'
elif 30 < body_mass_index <= 35:
    category = 'moderately obese'
elif 35 < body_mass_index <= 40:
    category = 'severely obese'
elif body_mass_index > 40:
    category = 'very severely obese'

print('Your BMI = ' + str(body_mass_index) + ' You are ' + category + '.')
input('Please press Enter to exit')

Which can still be improved.


Mutually exclusive conditions

Because of the way we check body_mass_index, if it is under 15, we get into the first case so there is no need to check elif 15 <= body_mass_index in the else part. This is guaranteed to be always true. Similarly, half the checks have no effect.

Code organisation

To make the code easier to understand (and easier to reuse, to test, etc), it is a good habit to split in into smaller reusable chunks. In our case, defining functions could be a nice touch.

Disclaimer: Next paragraph can be a bit overwhelming for a beginner, do not worry if you do not fully get it. It highlights how to solve problems you may not be interested in yet but it is a good chance to do things properly.

Also, if we want to be able to actually reuse your functions, we want to be able to import the file. Currently, if we do so, we get stuck into the parts asking for user inputs. Thus, the usual strategy is the following: split your code into 2 parts:

  • code defining functions/constants/classes/etc but without any side-effect or user interactions

  • code actually doing things (input/output, etc) behind an if __name__ == "__main__": guard. As pointed out by Solomon Ucko, it's generally a good idea to have this performed via a main() function and have the last two (non-blank) lines be if __name__ == '__main__': main(). This allows the interactivity to be triggered as needed.


At this stage, we have:

INCHES_PER_FOOT = 12
METERS_PER_INCH = 0.0254

def convert_feet_to_meter(height_in_feet):
    return height_in_feet * INCHES_PER_FOOT * METERS_PER_INCH

def get_body_mass_index(height_in_meter, weight_in_kilo):
    return weight_in_kilo / (height_in_meter ** 2)

def get_category(body_mass_index):
    if body_mass_index < 15:
        return 'very severely underweight'
    elif body_mass_index <= 16 :
        return 'severely underweight'
    elif body_mass_index <= 18.5:
        return 'underweight'
    elif body_mass_index <= 25:
        return 'Normal(healthy weight)'
    elif body_mass_index <= 30:
        return 'overweight'
    elif body_mass_index <= 35:
        return 'moderately obese'
    elif body_mass_index <= 40:
        return 'severely obese'
    else:
        return 'very severely obese'

def main():
    weight_in_kilo = float(input('Enter your weight in kilo:'))
    height_in_feet = float(input('Enter your height in feet:'))
    height_in_meter = convert_feet_to_meter(height_in_feet)
    body_mass_index = get_body_mass_index(height_in_meter, weight_in_kilo)
    category = get_category(body_mass_index)
    print('Your BMI = ' + str(body_mass_index) + ' You are ' + category + '.')
    input('Please press Enter to exit')

if __name__ == "__main__":
    main()

Going further, a few more details could be improved.

String formatting

Python offers many tools to format strings so that you do not need to use string concatenations. You can refer to PyFormat.info for documentations and examples.

You could write:

print('Your BMI = %f You are %s.' % (body_mass_index, category))

Or the newer technique:

print('Your BMI = {} You are {}.'.format(body_mass_index, category))

Also, in Python 3.6, yet another soution was added: F-strings.

Data over code

This may be a bit overkill here but sometimes a lot of code can be replaced by a small amount of code working on a properly filled data structure. In our case, the get_category function does the same thing for all categories: check the if we are under a given limit and if so, return the category name.

Disclaimer: Next part works under the assumption that body_mass_index < 15 should actually use <= like the other cases.

# List of pairs (higher-limit, name) sorted
CATEGORIES = [
    (15, 'very severely underweight'),
    (16, 'severely underweight'),
    (18.5, 'underweight'),
    (25, 'Normal(healthy weight)'),
    (30, 'overweight'),
    (35, 'moderately obese'),
    (40, 'severely obese'),
]

def get_category(body_mass_index):
    for limit, name in CATEGORIES:
        if body_mass_index <= limit:
            return name
    return 'very severely obese'
\$\endgroup\$
  • 1
    \$\begingroup\$ I haven't written python for a few years, but couldn't you only use if and return with a response function as soon as a condition is met? \$\endgroup\$ – Džuris Dec 30 '18 at 15:09
  • \$\begingroup\$ @Džuris I am not quite sure what you mean. Could you write somewhere ( pastebin.com ?) what you have in mind ? \$\endgroup\$ – Josay Dec 30 '18 at 15:13
  • \$\begingroup\$ sorry, I somehow misread a sample and was suggesting an idea similar to what's already there. Now that I read it properly on a computer, my question boils down to: Why do you need elif (instead of if) and else in the second sample? Isn't it that a later condition is only reached if none of the previous are met and just if (or nothing for the last case) would be enough? \$\endgroup\$ – Džuris Dec 30 '18 at 17:43
  • \$\begingroup\$ @Džuris Indeed, it would be the same. I kept the original elif because it didn't bother me. Then it is just a matter of personal preference. I have no strong opinion on this. \$\endgroup\$ – Josay Dec 30 '18 at 18:00
  • 1
    \$\begingroup\$ It's generally a good idea to have a main() function and have the last two (non-blank) lines be if __name__ == '__main__': main(). This allows the interactivity to be triggered as needed. \$\endgroup\$ – Solomon Ucko Dec 31 '18 at 3:26
3
\$\begingroup\$

One big refactoring that you could do is to remove all the if/else causes.

For example:

def compute_bmi(weight_in_kilo, height_in_feet):
    STUPID_CONVERSION_CONSTANT = 12 * 0.025
    return weight_in_kilo / ((height_in_feet * STUPID_CONVERSION_CONSTANT) ** 2)

def find_key_from_bmi(bmi):
    keys = list(bmi_info.keys())
    mins = [k if k <= bmi else 0  for k in keys]
    return keys[mins.index(min(mins))]

def print_message(bmi):
    print(f"Your BMI is {bmi} which means you are {bmi_info[find_key_from_bmi(bmi)]}.")

bmi_info = {
    15: 'very severely underweight',
    16: 'severely underweight',
    18.5: 'underweight',
    25: 'normal (healthy weight)',
    30: 'overweight',
    35: 'moderately obese',
    40: 'severely obese',

}

print_message(compute_bmi(float(input("Enter you weight in kilo:")), float(input("Enter your height in feet:"))))

This scales to an arbitrary large number of categories (possibly automatically generated) without the need to write extra code.

\$\endgroup\$
  • \$\begingroup\$ Why do you import numpy? Also, it's probably best to define the constant outside the loop and with a descriptive name such as METERS_PER_FOOT. \$\endgroup\$ – Solomon Ucko Jan 1 at 2:33
  • \$\begingroup\$ Numpy was a leftover for no valid reason. \$\endgroup\$ – Cedric H. Jan 1 at 11:33
2
\$\begingroup\$

Well, you could refactor the print part and extract it to a function like this:

def printBMIMessage(bodyMassIndex, message):
     print('Your BMI = ' + str(bodyMassIndex) + ' ' + message)


print('Enter your weight in kilo:')
weightInKilo = float(input())

print('Enter your height in feet:')

heightInFeet = float(input())

heightInMeter = heightInFeet * 12 * 0.025

bodyMassIndex = weightInKilo / (heightInMeter ** 2)

if bodyMassIndex < 15:
    printBMIMessage(bodyMassIndex, 'You are very severely underweight.')

elif bodyMassIndex >= 15 and bodyMassIndex <= 16 :
     printBMIMessage(bodyMassIndex, 'You are severely underweight.')

elif bodyMassIndex > 16 and bodyMassIndex <= 18.5:
    printBMIMessage(bodyMassIndex, 'You are underweight.')

elif bodyMassIndex > 18.5 and bodyMassIndex <= 25:
    printBMIMessage(bodyMassIndex, 'You are Normal(healthy weight).')

elif bodyMassIndex > 25 and bodyMassIndex <= 30:
    printBMIMessage(bodyMassIndex, 'You are overweight.')


elif bodyMassIndex > 30 and bodyMassIndex <= 35:
   printBMIMessage(bodyMassIndex, 'You are moderately obese.')

elif bodyMassIndex > 35 and bodyMassIndex <= 40:
    printBMIMessage(bodyMassIndex, 'You are severely obese.')

elif bodyMassIndex > 40:
   printBMIMessage(bodyMassIndex, 'You are very severely obese.')

input('Please press Enter to exit')
\$\endgroup\$
  • \$\begingroup\$ I don't really like the control flow of the printBMIMessage function because it allows to do printBMIMessage(40, 'You are underweight'). \$\endgroup\$ – svavil Dec 30 '18 at 19:59

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.