Computing the BMI

Question

The is my code to compute BMI (body mass index).

How can I make this code better?

const computeBMI = (weight, height, country) => {
 const countries = ["Chad", "Sierra Leone", "Mali", "Gambia", "Uganda", "Ghana", "Senegal", "Somalia", "Ivory Coast", "Isreal"];
 let heightInMeters = height * 0.3048;
 let BMI = weight / (heightInMeters * heightInMeters);
 for (let i = 0; i < countries.length; i++) {
 if (countries[i] === country) {
 const bmiTotal = BMI * 0.82;
 return Math.round(bmiTotal, 2);
 }
 }
 return Math.round(BMI, 2);
};

Answer 1

Your code looks quite good already. However there are a few details to improve:

1) There is Array.includes which will abstract away the for loop, making the code more concise.

2) You use bmiTotal and BMI, although both mean the same.

 const countriesWithLowerBIP = ["Chad", "Sierra Leone", "Mali", "Gambia", "Uganda", "Ghana", "Senegal", "Somalia", "Ivory Coast", "Isreal"];
 const computeBMI = (weight /*in kg*/, height /*in feet*/, country) => {
 const heightInMeters = height * 0.3048;
 let BMI = weight / (heightInMeters ** 2);
 if (countriesWithLowerBIP.includes(country)) 
 BMI *= 0.82;
 return Math.round(BMI, 2);
 };

• MikeT: odd i could have sworn you only had a single asterisk i must have misread

Answer 2

Avoid magic numbers. That's 0.3048 and 0.82. When another developer comes in and looks at the code, they won't know what those numbers are. Put them in variables that are named accordingly.

You converted height into meters, which implies that height isn't in meters. What unit does it come in? Is it some other metric unit or is it another unit? You didn't convert weight. Is weight already in metric? Would be better to use units as argument names instead.

Also, stick to one unit for calculation. Move conversions elsewhere. For instance, write in metric for everything. Then create separate APIs for other units, doing the conversion to metric there, then call the metric APIs.

That loop is a long-winded way to modify BMI for specific countries. It took me a while to figure that out. Would be nice to add comments on what that entire block does. Also, there's array.includes().

Lastly, any constant values in a function (values that literally never change, like your countries list and ratio), you should pull out of the function. Keeps the function less cluttered. Also, you wouldn't want to recreate those values every time the function is called.

const adjustedRatio = 0.82
const countries = [...];
const computeBMI = (kilograms, meters, country) => {
 const baseBMI = weight / (meters * meters);
 const shouldRatio = countries.includes(country)
 const modifiedBMI = shouldRatio ? baseBMI * adjustedRatio : baseBMI
 return Math.round(modifiedBMI, 2);
};
// For non-metric users
const computeBMINonMetric = (pounds, feet, country) => {
 // TODO: convert pounds to kilograms
 // TODO: convert feet to meters
 return computeBMI(kilograms, meters, country)
}

Answer 3

Some points

• Put magic numbers in constants to give them meaning.
• Variables that do not change should be defined as constants
• Use appropriate Array functions rather than iterating over the array.
• Math.round only takes one argument. I do not know why you pass 2 as a second argument. If I guess you want to round to 2 decimal places. To do that you can use Number.toFixed() however that returns the number as a String so to be safe and return the same type you would use Number(BMI.toFixed(2)) or you can use Math.round(BMI * 100) / 100
• You can use the ** operator to raise a value to a power.

Examples

const computeBMI = (weight, height, country) => {
 const FEET_2_METERS = 0.3048;
 const BMI_SCALE = 0.82;
 const COUNTRIES = ["Chad", "Sierra Leone", "Mali", "Gambia", "Uganda", "Ghana", "Senegal", "Somalia", "Ivory Coast", "Isreal"];
 return Math.round(weight / (height * FEET_2_METERS) ** 2 *
 (COUNTRIES.includes(country) ? BMI_SCALE : 1));
 
};

Though it would be better to have the constants held in closure if you are to use them in other functions.

const compute = (()=> {
 const FEET_2_METERS = 0.3048;
 const BMI_SCALE = 0.82;
 const COUNTRIES = ["Chad", "Sierra Leone", "Mali", "Gambia", "Uganda", "Ghana", "Senegal", "Somalia", "Ivory Coast", "Isreal"]; 
 return {
 BMI(weight, height, country) {
 return Math.round(weight / (height * FEET_2_METERS) ** 2 * (COUNTRIES.includes(country) ? BMI_SCALE : 1));
 }
 };
})();
compute.BMI(90, 6.2, "Somewhere");

Answer 4

This is pretty much how I would do it. Only part that you can trim down a bit is the use of for loop for finding out if the person is from the list of countries. There you can use countries.indexOf(country) that returns the index of the country in the array or -1 if the country is not in the array. You can use that in the if clause.

Otherwise it looks good to me.