Does everything look alright?

Is everything here correct logically and conventionally?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
//Ashton Dreiling
//BMI Exercise
#include <iostream>
#include <stdlib.h>

using namespace std;
//module prototype
void calculatesBmi(double &BMI,double weight, double height);

//Global constants
const int NUMBER = 702;
const double NUMBERFORBMIRANGE = 18.5;
const int SECONDNUMBERFORBMIRANGE = 25;

int main()
{
    //some variables
    double weight;
    double height;
    double BMI;

    cout << "Hello, today we are going to be calculating your BMI" << endl;
    cout << "Please type in your weight in pounds" << endl;
    cin >> weight;
    cout << "You typed in " << weight << endl;
    cout << "Please type in your height in inches." << endl;
    cin >> height;
    cout << "You typed in " << height << endl;
    cout << "Please give us a second to calculate your BMI." << endl;

    calculatesBmi(BMI,weight,height);
    //pass variables to calculatesBmi module
    cout << "Your BMI is " << BMI << endl;

    //determine whether whether the user is underweight, overweight, or optimal
    if ( (BMI > NUMBERFORBMIRANGE) && (BMI < SECONDNUMBERFORBMIRANGE))
        cout << "You are in optimal range!" << endl;
    else
    {
        if (BMI<NUMBERFORBMIRANGE)
        cout << "You are underweight. Are you okay? Eat better!" << endl;
        else
        {
            if (BMI>SECONDNUMBERFORBMIRANGE)
                cout << "You are overweight. It's okay to like your body, but stay healthy, too! Exercise, it's good for you!" << endl;
        }

    }

    system("Pause");


    return 0;
}//end main

void calculatesBmi(double &BMI,double weight, double height)
{   //calculating BMI

    BMI = weight / (height * height) * NUMBER;
}//end calculatesBmi

(1) Dont use system

(2) Forget about using namespace std. Instead use std:: (std followed by scope resolution operator)

(3) I would suggest changing the if followed by nested else's to if followed by else-if chain, like below:

1
2
3
4
5
6
7
8
    if ( (BMI > NUMBERFORBMIRANGE) && (BMI < SECONDNUMBERFORBMIRANGE))
        std::cout << "You are in optimal range!" << std::endl;
   
    else if (BMI < NUMBERFORBMIRANGE)
        std::cout << "You are underweight. Are you okay? Eat better!" << std::endl;
    
    else if (BMI > SECONDNUMBERFORBMIRANGE)
        std::cout << "You are overweight. It's okay to like your body, but stay healthy, too! Exercise, it's good for you!" << std::endl;


(4) Some comments are a bit overkill. No need to write 'end function' comments. Comments on lines 17, 32 and 57 are unnecessary.
I have to use system. Instructor requires it.

I have to use using namespace std as well.

That's a good suggestion; perhaps I'll use it later down the line when my teacher allows it. : )

They're necessary in his class or else we lose points.
closed account (48bpfSEw)
pretty nice.

what if the user don't understand english?
extract all your text in a language-file!

What mean your programm with "eat better"?
What is "better"?

If you want to write a better diagnostic programm,
you could follow the way of ayurveda
with its three doshas.

https://en.wikipedia.org/wiki/Dosha

every person has its own combination of these three doshas.
with a palett of questions one can figure out which of the doshas is not in harmony and can give certain recommendations for changing food to get in harmony again.

e.g. http://www.naturesformulary.com/contents/dosha-test

Ah, okay. Well, if your teacher requires it, better to play along. When you are done with the course, look up and try to implement better programming practices. They may seem small, but take you far.
Thanks guys! All great advice!
Also, avoid global variables, at least put them in main. They might not seem so bad in a small program, but before long they get out of hand. The golden rule is to keep the scope as small as possible.

Another golden rule is to always initialise your variables, preferably wait until you have a sensible value to assign to it, then do the declaration and assignment all in one statement. Otherwise initialise it to something valid but odd looking, for example 999.0 . Note zero is not always a good choice. I always put digits before and after the decimal place for doubles - it reinforces the idea that it is a double, and will never be interpreted by the compiler as an int. That can lead to integer division errors (not here though, just saying). The fact that you have two of your constants as ints does unnecessary implicit casts of those ints to double later in the program.

Try to choose good meaningful variable names. NUMBER is not such a good choice IMO, it's really a conversion factor because the units aren't metric. Similar idea for the other 2 constants, they are really the lower and upper limits for the BMI range

One more pedantic thing: One can build up a std::cout statement in stages, no need to do all in one hit:

1
2
cout << "You are overweight. It's okay to like your body, but stay healthy, too! ";
 cout << "Exercise, it's good for you!" << endl;


Alternatively, you can have each line start with << :

1
2
cout << "You are overweight. It's okay to like your body, but stay healthy, too! "
     << "Exercise, it's good for you!" << endl;


One can split statements over several lines, all the compiler cares about is the semi-colon.

Hopefully this has been helpful :+)
Last edited on
Thank you. <3
Topic archived. No new replies allowed.