How can I make this code simpler in kotlin?-CodePudding

I learned to program by myself and just decided to go with Kotlin.

I made a simple program that solves a triangle which works well but my code is ugly and repetitive.

Can you help me on how to improve it, any suggestion will help since even my code works it does not look correct.

    private fun validateNumbers() {
        if(binding.etHip.text.toString().isNotEmpty()){
            Hip = (binding.etHip.text.toString()).toFloat()
        }
        if(binding.etAdj.text.toString().isNotEmpty()){
            Adj = (binding.etAdj.text.toString()).toFloat()
        }
        if(binding.etOpp.text.toString().isNotEmpty()){
            Opp = (binding.etOpp.text.toString()).toFloat()
        }
        if(binding.etAngle.text.toString().isNotEmpty()){
            Angle = (binding.etAngle.text.toString()).toFloat()
        }

    }


    private fun countSelected() {
        binding.cbHyp.setOnCheckedChangeListener { compoundButton, b ->
            if(binding.cbHyp.isChecked){
                numberCB  
            }
            else{
                numberCB--
            }
        }

        binding.cbAdj.setOnCheckedChangeListener { compoundButton, b ->
            if(binding.cbAdj.isChecked){
                numberCB  
            }
            else{
                numberCB--
            }
        }

        binding.cbOpp.setOnCheckedChangeListener { compoundButton, b ->
            if(binding.cbOpp.isChecked){
                numberCB  
            }
            else{
                numberCB--
            }
        }

        binding.cbAngle.setOnCheckedChangeListener { compoundButton, b ->
            if(binding.cbAngle.isChecked){
                numberCB  
            }
            else{
                numberCB--
            }
        }


    }

CodePudding user response：

with a extension function for input text. and if is a radio group. is a function about listener the radio grup instead doing for each one

CodePudding user response：

How about this? The logic is the same. it's only simplified syntactically.

private fun validateNumbers() = with(binding) {
    if (etHip.text.toString().isNotEmpty()) Hip = (etHip.text.toString()).toFloat()
    if (etAdj.text.toString().isNotEmpty()) Adj = (etAdj.text.toString()).toFloat()
    if (etOpp.text.toString().isNotEmpty()) Opp = (etOpp.text.toString()).toFloat()
    if (etAngle.text.toString().isNotEmpty()) Angle = (etAngle.text.toString()).toFloat()
}

private fun countSelected() = with(binding) {
    setOf(cbHyp, cbAdj, cbOpp, cbAngle).forEach {
        it.setOnCheckedChangeListener { if (it.isChecked) numberCB   else numberCB-- }
    }
}

CodePudding user response：

For both functions, you can wrap them in with(binding) so you don't have to keep writing binding.

For validateNumbers, you can use toFloatOrNull() to convert values to Float, or null if it doesn't evaluate as a Float (like an empty string would be invalid). Then you can use the Elvis operator ?: to provide a default. So if you don't want the value to change, it can return itself.

Also, since you're doing it repeatedly, you can make a shortcut for getting the value from an EditText. which is a type of TextView, so you can write it as an extension of TextView.

private fun TextView.toFloatOrNull() = text.toString().toFloatOrNull()

private fun validateNumbers() = with(binding) {
    Hip = etHip.toFloatOrNull() ?: Hip
    Adj = etAdj.toFloatOrNull() ?: Adj
    Opp = etOpp.toFloatOrNull() ?: Opp
    Angle = etAngle.toFloatOrNull() ?: Angle
}

With countSelected() since they're all doing exactly the same thing in their listeners, you can give them all the same listener. The second parameter in the lambda it whether it was just checked, so you can simplify that logic a bit too. You can use listOf(...).forEach to get them all.

fun countSelected() = with(binding) {
    val listener = OnCheckedChangeListener { _ isChecked ->
        if (isChecked) numberCB   else numberCB--
    }
    listOf(cbHyp, cbAdj, cbOpp, cpAngle).forEach {
        it.onCheckedChangeListener = listener
    }
}

I would also change the function name to something more descriptive. Your function isn't counting something...it's setting up the checkboxes to count themselves.

Note, property and variable names should start with lowercase letters by convention. It will make your code easier to read. Uppercase first letters are generally reserved for class and interface names. (They are also used for function names in some other languages, but you shouldn't do that in Kotlin or they'll get confused with constructor calls.)