Body Mass Index Calculator

I created a quick calculator in Javascript and I was wondering if there was a way to make the code look prettier or make it simpler. To me, it looks really cramped and messy and I'm not sure how change that.

function calc() {
    var height = (document.getElementById('height').value) / 100;
    var weight = document.getElementById('weight').value;

    var output = document.getElementById('output');
    var bmi = Math.round((weight / Math.pow(height, 2)) * 10) / 10;

    var result = document.getElementById('result');
    var text = "";
    if (height != "" && weight != "") {
        if (bmi != "NaN" && bmi != 0) {
            if (bmi % 1 != 0) {
                output.innerHTML = bmi;
            } else {
                output.innerHTML = bmi + ".0";
            }
            if (10 <= bmi && bmi < 14) {
                text = "You are seriously underweight!";
            } else if (15 <= bmi && bmi < 18.5) {
                text = "You are slightly underweight.";
            } else if (18.5 <= bmi && bmi < 25) {
                text = "You are healthy!";
            } else if (25 <= bmi && bmi < 30) {
                text = "You are slightly overweight.";
            } else if (30 <= bmi && bmi < 40) {
                text = "You are obese!";
            } else if (40 <= bmi && bmi < 50) {
                text = "You are seriously obese!";
            } else {
                text = "You should be dead...";

            }

            result.innerHTML = text;
        }
    }
}

The reason it seems messy is because you have one function doing multiple things. So what does your function do?

• It queries the DOM for elements containing the input.

• It calculates the BMI based on that input

• It has logic that determines what kind of output to produce

• It produces output to the DOM.

There are a few problems with that:

• What if you want to run this function in a different DOM? You can't, you're bound to elements with specific IDs.

• What if you want just the result? You can't, because your function doesn't return anything but explicitly mutates the DOM.

• It's unclear from the signature what your function needs to work (i.e. available DOM with elements with specific IDs). Functions normally declare what they need in the form of parameters.

So how can we do better? We can divide into several independent functions:

function calculateBMI(height, weight) {
  // todo
  // calculate BMI based on height and weight passed in.
  // returns a number (the BMI)
}

function bmiToHealth(bmi) {
  // todo
  // determine what text to display based on the bmi value.
  // returns a string (the text to display)
}

And use it like this:

function calculateBmiFromDOM() {
    let heightInput = document.getElementById('height');
    let weightInput = document.getElementById('weight');
    let output = document.getElementById('output');
    let result = document.getElementById('result');

    let bmi = calculateBMI(heightInput.value, weightInput.value);
    output.textContent = bmi.toPrecision(3); // 25 -> 25.0, 23.752 -> 23.7
    result.textContent = bmiToHealth(bmi);
}
let form = getFormSomehow(); // usually with getElementById or querySelector
// You can pass functions as parameters in JavaScript
form.addEventListener('submit', calculateBmiFromDOM);

And that last snippet the only thing that's specific to your current DOM, so when your DOM changes, you don't need to change all of your code, just the bit that interacts with it.

Additional notes for future snippets (just general things I've noticed):

• anything != 'NaN' is pointless, nothing in your logic might return a string with the value of 'NaN'. Even if it were NaN, you can't compare it with ==, you need to use the isNaN() function.

• You should always use === or !== when comparing. Using == and != can introduce unexpected behavior due to type conversion.

• Functions work best when they accept values as arguments and/or return values as result.

StackExchange Code Review Q#116406, answer score: 20