4
\$\begingroup\$

I have made a Body mass index-calculator using ViewModel and LiveData.

Screenshot 1

Screenshot 2

Source-code of the MainActivity:

package com.example.bmirechner
import androidx.appcompat.app.AppCompatActivity
import android.os.Bundle
import android.widget.SeekBar
import androidx.lifecycle.Observer
import androidx.lifecycle.ViewModelProvider
import com.example.bmirechner.databinding.ActivityMainBinding
class MainActivity : AppCompatActivity() {
 private lateinit var binding: ActivityMainBinding
 private lateinit var viewModel: BmiViewModel
 fun updateUi() {
 val bmi = viewModel.computeBmi()
 binding.resultTextView.text = bmi.toString()
 var messageText = ""
 when {
 bmi < 20 -> messageText = resources.getString(R.string.text_underweight)
 bmi in 20.0..25.0 -> messageText = resources.getString(R.string.text_normal_weight)
 bmi > 25 && bmi <= 30 -> messageText = resources.getString(R.string.text_overweight)
 bmi > 30 -> messageText = resources.getString(R.string.text_heavy_overweight)
 else -> messageText = resources.getString(R.string.error)
 }
 binding.messageTextView.text = messageText
 }
 override fun onCreate(savedInstanceState: Bundle?) {
 super.onCreate(savedInstanceState)
 setContentView(R.layout.activity_main)
 binding = ActivityMainBinding.inflate(layoutInflater)
 setContentView(binding.root)
 viewModel = ViewModelProvider(this).get(BmiViewModel::class.java)
 binding.currentWeightTextView.text = viewModel.getCurrentWeight().value.toString()
 binding.currentHeightTextView.text = viewModel.getCurrentHeight().value.toString()
 binding.heightSeek.setOnSeekBarChangeListener(object: SeekBar.OnSeekBarChangeListener {
 override fun onProgressChanged(seekBar: SeekBar?, progress: Int, fromUser: Boolean) {
 viewModel.setCurrentHeight(progress.toDouble())
 viewModel.getCurrentHeight().observe(this@MainActivity, Observer {
 binding.currentHeightTextView.text = it.toString()
 })
 }
 override fun onStartTrackingTouch(seekBar: SeekBar?) {}
 override fun onStopTrackingTouch(seekBar: SeekBar?) {
 updateUi()
 }
 })
 binding.weightSeek.setOnSeekBarChangeListener(object: SeekBar.OnSeekBarChangeListener {
 override fun onProgressChanged(seekBar: SeekBar?, progress: Int, fromUser: Boolean) {
 viewModel.setCurrentWeight(progress.toDouble())
 viewModel.getCurrentWeight().observe(this@MainActivity, Observer {
 binding.currentWeightTextView.text = it.toString()
 })
 }
 override fun onStartTrackingTouch(seekBar: SeekBar?) {}
 override fun onStopTrackingTouch(seekBar: SeekBar?) {
 updateUi()
 }
 })
 }
}

Source-Code of the ViewModel:

package com.example.bmirechner
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.ViewModel
import kotlin.math.pow
import kotlin.math.round
class BmiViewModel: ViewModel() {
 private var currentHeight = MutableLiveData<Double>()
 private var currentWeight = MutableLiveData<Double>()
 init {
 currentHeight.value = 0.0
 currentWeight.value = 0.0
 }
 fun setCurrentHeight(value: Double) {
 currentHeight.value = value
 }
 fun setCurrentWeight(value: Double) {
 currentWeight.value = value
 }
 fun getCurrentHeight(): MutableLiveData<Double> {
 return currentHeight
 }
 fun getCurrentWeight(): MutableLiveData<Double> {
 return currentWeight
 }
 fun computeBmi(): Double {
 val factor = 10
 val heightInMeter = currentHeight.value!! / 100.0
 val bmi = currentWeight.value!! / heightInMeter.pow(2)
 return round(bmi * factor) / factor
 }
}

Is my LiveData-usage correct or should it be modified?

What parts of my implementation could be improved?

Looking forward to reading your comments and answers.

Peilonrayz
44.4k7 gold badges80 silver badges157 bronze badges
asked Dec 28, 2021 at 10:44
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Perhaps add instruction related to units used for height/weight :) (cm/kg) \$\endgroup\$ Commented Dec 28, 2021 at 12:20
  • \$\begingroup\$ @hjpotter92 Thanks. I really should have done so. \$\endgroup\$ Commented Dec 28, 2021 at 13:02
  • \$\begingroup\$ i would appreciate if you had put the actions from your slider into a method and could avoid the redundant code. (that far to little for a whole review - its just a side note ^^) \$\endgroup\$ Commented Feb 14, 2022 at 19:37

1 Answer 1

3
\$\begingroup\$

unit confusion

I found the screenshot slightly jarring, and not only because Europeans will seldom cite their height with millimeter accuracy. (The 1.8m might be better displayed as integer cm.)

To view unlabeled / unitless quantities in the context of BMI, my mind immediately went to "oh, we're going to have m and kg." But then I had trouble envisioning a 180 m tall human, and revised downward to cm, which curiously has four, count 'em, four sig figs, hmmm.

Either display \1ドル.80\$ m, or offer on-screen "m" & "kg" labels.

meaningful identifier names

The BmiViewModel attribute names are not good.

 fun computeBmi(): Double {
 val factor = 10
 val heightInMeter = currentHeight.value!! / 100.0
 val bmi = currentWeight.value!! / heightInMeter.pow(2)
 return round(bmi * factor) / factor
 }

In any codebase, physical quantities should default to SI (MKS), unless documented as e.g. CGS or imperial measure. So currentWeight is the perfect identifier.

 val heightInMeter = currentHeight.value!! / 100.0

Here, the second identifier is just wrong, as evidenced by the need to apply a conversion factor. It should be renamed, or better, it should store some number of meters, with no need for conversion. Then the UI can convert to convenient units as needed.

historic background

Unit confusion has led to numerous code defects, including loss of the Mars Climate Orbiter.

software that calculated the total impulse produced by thruster firings produced results in pound-force seconds. The trajectory calculation software then used these results – expected to be in newton-seconds (incorrect by a factor of 4.45) – to update the predicted position of the spacecraft.

Do not write the next such piece of code.

range comparison

The updateUI() code could be DRY'd up a bit.

 when {
 bmi < 20 -> messageText = resources.getString(R.string.text_underweight)
 bmi in 20.0..25.0 -> messageText = resources.getString(R.string.text_normal_weight)
 bmi > 25 && bmi <= 30 -> messageText = resources.getString(R.string.text_overweight)
 bmi > 30 -> messageText = resources.getString(R.string.text_heavy_overweight)
 else -> messageText = resources.getString(R.string.error)
 }

Use parallel construction to phrase parallel tests, rather than a .. range here and a && conjunct there. Perform these comparisons:

  • bmi < 20
  • bmi < 25
  • bmi < 30

Consider breaking this out as a trivial helper function, which can do an early return once it finds an answer. Then you could write a single resources.getString() call on the result.

It's unclear how the bmi > 30 and the else clauses could both be relevant. Whoever winds up measuring unit test coverage is going to have a tough time covering both those source lines.

answered Mar 9 at 16:58
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.