BMI calculator C code

Question

I'm trying to write a simple BMI calculator, but for some reason when I try 175 for height (which the formula makes 1.75) and 70 for mass it should give 22.8, which is in the healthy range, but it gives me underweight. I know it's probably a simple mistake, but I can't see it.

float main(void) { 

    float height;   
    printf("Enter your height in cm:\n");
    scanf("%f",&height);    

    float weight; 
    printf("Enter your weight in kg:\n");
    scanf("%f",&weight);

    float bmi;
    bmi = (weight/(height/100)*(height/100));

    if (bmi <= 16) {
        printf("Severely Underweight\n");
    }
    else if (16 < bmi <= 18.5) {
        printf("Underweight\n");
    }
    else if (18.5 < bmi <= 25) {
        printf("Healthy\n");
    }
    else if (25 < bmi <= 30) {
        printf("Overweight\n");
    }
    else {
        printf("Severely Overweight\n");
    }
}

Bathsheba has it, but you don't need compound conditions in the first place, because the conditions occur in `else if`s, where the lower bound is already ensured. — M Oehm, Feb 12 '16 at 10:17
Neither C, nor C++ supports operator chaining like that. Divide the condition into two, seperated by the logical AND operator to get the desired result. — Spikatrix, Feb 12 '16 at 10:19
I like a bmi calculator that always says I'm not overweight. Compile, and ship it! — Bathsheba, Feb 12 '16 at 10:20
It's worse, it always says I'm super-obese, because the calculation isn't right: The supposed denominators cancel each other out and the weight in kg is the result. (Funny how the formula uses many parentheses, but misses the only important pair around the denominator.) — M Oehm, Feb 12 '16 at 10:24
I'm assuming `float main(void)` is one of the freestanding implementations, likely a maritime application? Where a hefty BMI of the sailors may have a drastic impact on the system's float performance. — Lundin, Feb 12 '16 at 10:41
And one more note in addition to answers that whenever you are dealing with float and integer constant together keep practice of writing integer as k.0 rather than simply writing k(16.0 not 16). It's a good practice and saves you from logical error. — Kevin Pandya, Feb 12 '16 at 10:48
@Bathsheba: Is that the software reuse everyone talks about? If it doesn't work, reuse it somewhere else? — M Oehm, Feb 12 '16 at 11:02
@MOehm and Bathsheba check my STL solution out (you guys seem to be having fun here anyway) — iksemyonov, Feb 12 '16 at 11:10

score 8 · Accepted Answer · edited May 23 '17 at 11:59

All of these

else if (16 < bmi <= 18.5) {

are wrong. They don't do what you mean them to do. To achieve the desired result, use

else if (16 < bmi && bmi <= 18.5) {

The reason is, your expressions are evaluated as

else if ((16 < bmi) <= 18.5) {

where (16 < bmi) evaluates to true or false which in turn equals 1 or 0, and is then compared to the second constant. Reason why it is evaluated like so, is because the comparison operators are left-associative, thus are evaluated left-to-right.

Edit 2

Obligatory SO link: Is (4 > y > 1) a valid statement in C++? How do you evaluate it if so?

Edit

I suspected this one, but didn't know the formula. Now @MOehm has confirmed it (and wikipedia seems to confirm this as well):

bmi = (weight/(height/100)*(height/100));

should become

bmi = (weight/((height/100)*(height/100)));

The reason here is pretty much the same: operator precedence and expression evaluation rules in C++. OP, pay attention to these aspects and put parentheses where appropriate!

Edit 3 Here's how I'd go about this using STL (this approach has the benefit of clearly expressing the idea behind the algorithm, without burying it beneath the implementations details):

#include <iostream>
#include <string>
#include <vector>
#include <utility>
#include <limits>
#include <algorithm>

int main()
{
    std::vector<std::pair<float, std::string> > bmi_table = {
        { 16, "Severely Underweight" },
        { 18.5, "Underweight" },
        { 25, "Healthy" },
        { 30, "Overweight" },
        { std::numeric_limits<float>::max(), "Severely Overweight" }
    };
    float height, weight;
    std::cin >>  height >> weight;
    const float bmi = (weight/((height/100.f)*(height/100.f)));
    const auto idx =
        std::find_if(bmi_table.begin(),
                     bmi_table.end(),
                     [&](decltype(bmi_table)::value_type& p) -> bool { return p.first > bmi; });
    std::cout << idx->second << '\n';
    return 0;
}

I was intentionally wooly in my answer due to the OPs multitagging. In C, the values of the relational operators are 1 and 0 and these are `int` types. In C++, they are `true` and `false` and are `bool` types. These are small but important differences that *can* be relevant when "chaining" operators. — Bathsheba, Feb 12 '16 at 10:41
I don't know C well, unfortunately. But then, aren't the `bool`s implicitly converted to `int`'s in C++? — iksemyonov, Feb 12 '16 at 10:44
Is there anything regarding this topic that I should correct in the answer? Gladly will do so. — iksemyonov, Feb 12 '16 at 10:51
Thank you, you've outdone yourself for the question and for what I was asking for. — Kevin Thomson, Feb 12 '16 at 11:35
@KevinThomson in fact, my comment was addressed to Bathsheba, forgot to add the @ notification, about whether I should fix the `bool`-related part of the answer. anyway, you're welcome. — iksemyonov, Feb 12 '16 at 18:02

Bathsheba · Answer 2 · 2016-02-12T10:22:20.783

6

16 < bmi <= 18.5 doesn't do what you think it does, for example. (Although it compiles, it actually gets evaluated as (16 < bmi) <= 18.5 and the bit in parentheses is either 1 (true) or 0 (false).)

You need to write 16 < bmi && bmi <= 18.5.

But if you order your bmi limits, you don't need to repeatedly test the lower bound.

edited Feb 12 '16 at 10:22

answered Feb 12 '16 at 10:15

Bathsheba

231,907
34
361
483

BMI calculator C code

2 Answers2

Linked

Related