2
\$\begingroup\$

This is "homework" from a Swift course, which I'm currently taking. A mockup and the image-assets are provided by the instructor. We are supposed to create the layout and to write some appropriate code.

App View

Here's the code of my ViewController:

import UIKit

class ViewController: UIViewController {
    // MARK: Outlets
    @IBOutlet weak var nameText: UITextField!
    @IBOutlet weak var sizeText: UITextField!
    @IBOutlet weak var weightText: UITextField!

    @IBAction func clickHereButton(_ sender: UIButton) {
        let name = nameText.text!
        let size = Double(sizeText.text!)!
        let weight = Double(weightText.text!)!

        let bmi = weight / pow(size, 2)
        let bmiRounded = (round(bmi * 10) / 10)
        print(bmiRounded)

        let uac = UIAlertController(title: "BMI for \(name)", message: "Your Body-Mass-Index is \(bmiRounded)", preferredStyle: .alert)
        uac.addAction(UIAlertAction(title: "Okay", style: .default))
        self.present(uac, animated: true, completion: nil)
    }
}

What are your thoughts about my implementation? What would you have done differently and why?

\$\endgroup\$
3
  • \$\begingroup\$ Force-unwrapping Double(sizeText.text!) and Double(weightText.text!) calls for trouble. Use optional binding instead. You could also set the keyboard type and define some valid ranges for the text fields. \$\endgroup\$ Commented Dec 23, 2019 at 20:12
  • \$\begingroup\$ Should guard against "division by zero" in your bmi calculation. The app will crash if size entered is zero. Fun fact, zero is a valid dress size in the US. \$\endgroup\$ Commented Dec 25, 2019 at 21:08
  • \$\begingroup\$ I think you are missing check for validation of text value from UITextField and displaying appropriate message if it isn't correct. For example on iPad user will be able to enter special symbols, even if you set decimal keyboard for UITextField. Another thing that I would recommend - calculate BMI "live" when user type in value in text field \$\endgroup\$ Commented Jan 8, 2020 at 20:36

1 Answer 1

Reset to default
3
\$\begingroup\$

You are doing double force-unwraps, twice. This is really just asking for trouble 😬

As others have pointed out in their comments, there are a few edge cases you should guard against. Good thing is you can do both at the same time.

Additionally, you should avoid using shortened names for your variables. In Swift, we usually type everything out.

I've updated and annotated your code below, take a look;

@IBAction func clickHereButton(_ sender: UIButton) {

    // Start by ensuring we have actual values entered by the user
    guard let name = nameText.text, 
          let sizeString = sizeText.text,
          let weightString = weightText.text else {
        return
    }

    // Then, ensure the values are valid Doubles
    guard let size = Double(sizeString),
          let weight = Double(weightString) else {
        return
    }

    // Finally, ensure that size > 0
    guard size > 0 else {
        return
    }

    // We now are 100% safe and can start calculating our BMI
    let bmi = weight / pow(size, 2)
    let bmiRounded = (round(bmi * 10) / 10)

    // Even though this var name is longer, it's clearer what it is.
    let alertViewController = UIAlertController(title: "BMI for \(name)", message: "Your Body-Mass-Index is \(bmiRounded)", preferredStyle: .alert)

    // You can use the `.init(_:)` shorthand when a specific type is required
    alertViewController.addAction(.init(title: "Okay", style: .default))
    self.present(alertViewController, animated: true, completion: nil)
}

The only other thing I would improve is to move your actual BMI calculations to a specific function that takes in the size and weight parameters.

For the rest, it's pretty much all good.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.