Body mass index calculator in Swift

Question

Today I created my second app - a simple body mass index calculator. The BMI is calculated by the following formula:

$$\dfrac{\text{weight}}{\text{(height)}^2}$$

The general meaning of the resulting BMI values:

• \$<18.5\$ = underweight
• \$18.5-25\$ = normal weight
• \$25-30\$ = overweight
• \$>30\$ = obese

My goal was to improve my skills and make something useful at the same time. I'm sharing my code in hope to get some constructive feedback. Is there anything I could improve?

My model file:

import Foundation

struct CalculatorBrain {
let weightInKG: Double
let heightInCM: Double

init(weightInKG: String, heightInCM: String) {
    self.weightInKG = Double(weightInKG) ?? 0.0
    self.heightInCM = Double(heightInCM) ?? 0.0
}

func calcBmi() -> Double {
    return weightInKG / ((heightInCM / 100) * (heightInCM / 100))
}
}

The ViewController:

class ViewController: UIViewController {

@IBOutlet weak var weightTextField: UITextField!

@IBOutlet weak var heightTextField: UITextField!

@IBOutlet weak var resultTextField: UILabel!

@IBAction func calculateBtn(_ sender: Any) {
    guard let weightInKG = weightTextField.text,
        let heightInCM = heightTextField.text else {
            return
    }
    let cm = CalculatorBrain(weightInKG: weightInKG, heightInCM: heightInCM)
    let result = cm.calcBmi()
    resultTextField.text = "BMI: \(result)"
}

}

Answer 1

Let me just congratulate you on some really good naming! It's a great idea to put the units in the variable name. I just spent the better part of a day on a tight deadline chasing down a bug that ended up being a units issue. I felt so stupid. The only named I'd work on improving is result. Why not just call it bmi? result is a very generic name unless you're writing something like a square root function. It doesn't tell you what it's the result of.

It seems a little weird to me that init takes its parameters as Strings and immediately converts them to Double. Why not have the model object just take them as Double and let the controller convert them? If you ever reuse this class or expand it for a task where the data doesn't come from a text field (like maybe it comes from a database or the health app), you may not have the values as Strings. It seems most efficient to have the init method take them in the format they'll be stored in and let the caller worry about doing the conversion.

Indentation styles are a personal thing. My preference would be to indent everything between the open and close braces of a struct or class. It's a minor thing, but I feel it improves readability, especially if you end up needing to put more than one in the same header.

Answer 2

I have some minor tips about naming:

• weight is entirely different than mass, thus I advice you to change that, because some users might be annoyed by this common confusion, if it is present in the user interface of the app. See Mass versus weight

• CalculatorBrain does not really feel related to BMI. Maybe BMICalculator would be a better fit here

---

When it comes to your struct, if that is all you are planning to do with it, this gets quite useless. Instead, I would simply use a function, taking two Double parameters (to avoid String conversion). Also, I would not use ((heightInCM / 100) * (heightInCM / 100)), because it does an unnecessary multiplication between two known numbers, two of 100 in this case. This would be an example:

func calculateBMI(massInKilograms mass: Double, heightInCentimeters height: Double) -> Double {
    return mass / ((height * height) / 10000)
}

It is shorter and, in my opinion, more elegant and efficient. And now you do not need to initialize a struct and call one of its methods each time, instead you only need to call it:

calculateBMI(massInKilograms: x, heightInCentimeters: y)