I have a codebase where all my data calculations and data formatting occur within a single function. I have heard that it is best practice to have one task per function but I wonder if calculations, formatting, and display can fit into one function as data processing? Or should I separate them all?
Here is the function I am referring to:
private fun processData(data: ByteArray) {
progressBar.visibility = GONE
Log.d(TAG, "displayDiagnosticData: ")
val bmsVersionView = findViewById<TextView>(R.id.textview_bms_version)
val boardVersionView = findViewById<TextView>(R.id.textview_board_version)
val cellOneView = findViewById<TextView>(R.id.textview_cell_1)
val cellTwoView = findViewById<TextView>(R.id.textview_cell_2)
val cellThreeView = findViewById<TextView>(R.id.textview_cell_3)
val cellFourView = findViewById<TextView>(R.id.textview_cell_4)
val cellFiveView = findViewById<TextView>(R.id.textview_cell_5)
val cellSixView = findViewById<TextView>(R.id.textview_cell_6)
val cellSevenView = findViewById<TextView>(R.id.textview_cell_7)
val cellEightView = findViewById<TextView>(R.id.textview_cell_8)
val cellNineView = findViewById<TextView>(R.id.textview_cell_9)
val cellTenView = findViewById<TextView>(R.id.textview_cell_10)
val cellElevenView = findViewById<TextView>(R.id.textview_cell_11)
val cellTwelveView = findViewById<TextView>(R.id.textview_cell_12)
val cellThirteenView = findViewById<TextView>(R.id.textview_cell_13)
val cellFourteenView = findViewById<TextView>(R.id.textview_cell_14)
val packTotalView = findViewById<TextView>(R.id.textview_diagnostic_voltage)
val packSocView = findViewById<TextView>(R.id.textview_diagnostic_soc)
val chargeTempView = findViewById<TextView>(R.id.textview_charge_temp)
val dischargeTempView = findViewById<TextView>(R.id.textview_discharge_temp)
val chargeCurrentView = findViewById<TextView>(R.id.textview_diagnostic_charge_current)
// val dischargeCurrentView = findViewById<TextView>(R.id.textview_diagnostic_discharge_current)
val dischargeCircuitStateView = findViewById<TextView>(R.id.textview_discharge_circuit)
val chargeCircuitStateView = findViewById<TextView>(R.id.textview_charge_circuit)
val balanceCircuitStateView = findViewById<TextView>(R.id.textview_balance_circuit)
val emptyCircuitStateView = findViewById<TextView>(R.id.textview_empty_circuit)
val bmsVersion = data[0] (data[1] * 256)
val cellOne = data[2].toDouble() / 100 3.52
val cellTwo = data[3].toDouble() / 100 3.52
val cellThree = data[4].toDouble() / 100 3.52
val cellFour = data[5].toDouble() / 100 3.52
val cellFive = data[6].toDouble() / 100 3.52
val cellSix = data[7].toDouble() / 100 3.52
val cellSeven = data[8].toDouble() / 100 3.52
val cellEight = data[9].toDouble() / 100 3.52
val cellNine = data[10].toDouble() / 100 3.52
val cellTen = data[11].toDouble() / 100 3.52
val cellEleven = data[12].toDouble() / 100 3.52
val cellTwelve = data[13].toDouble() / 100 3.52
val cellThirteen = data[14].toDouble() / 100 3.52
val cellFourteen = data[15].toDouble() / 100 3.52
val totalVoltage = 47.8 (data[16].toDouble() / 10)
val chargeTempCelsius = data[19]
val dischargeTempCelsius = data[20]
val chargeTempFahr = (chargeTempCelsius * 9.0 / 5.0) 32.0
val dischargeTempFahr = (dischargeTempCelsius * 9.0 / 5.0) 32.0
val chargeCurrent = data[21]
// val dischargeCurrent = (data[23].toDouble() * 100 data[22]).toInt()
val chargeCircuitState = data[25].toInt()
val dischargeCircuitState = data[26].toInt()
val balanceCircuitState = data[27].toInt()
val emptyCircuitState = data[28].toInt()
val chargeCircuit: String = if (chargeCircuitState == 1) {
"On"
}else {
"Off"
}
val dischargeCircuit: String = if (dischargeCircuitState == 1) {
"On"
}else {
"Off"
}
val balanceCircuit: String = if (balanceCircuitState == 1) {
"On"
}else {
"Off"
}
val emptyCircuit: String = if (emptyCircuitState == 1) {
"On"
}else {
"Off"
}
val bmsVersionString = SpannableString("BMS Version: $bmsVersion")
bmsVersionString.setSpan(StyleSpan(Typeface.BOLD), 0, 11, 0)
val boardVersionString = SpannableString("Board Version: 2.1")
boardVersionString.setSpan(StyleSpan(Typeface.BOLD), 0, 13, 0)
val cellOneString = SpannableString("Cell 1: %.2fV".format(cellOne))
cellOneString.setSpan(StyleSpan(Typeface.BOLD), 0, 6, 0)
val cellTwoString = SpannableString("Cell 2: %.2fV".format(cellTwo))
cellTwoString.setSpan(StyleSpan(Typeface.BOLD), 0, 6, 0)
val cellThreeString = SpannableString("Cell 3: %.2fV".format(cellThree))
cellThreeString.setSpan(StyleSpan(Typeface.BOLD), 0, 6, 0)
val cellFourString = SpannableString("Cell 4: %.2fV".format(cellFour))
cellFourString.setSpan(StyleSpan(Typeface.BOLD), 0, 6, 0)
val cellFiveString = SpannableString("Cell 5: %.2fV".format(cellFive))
cellFiveString.setSpan(StyleSpan(Typeface.BOLD), 0, 6, 0)
val cellSixString = SpannableString("Cell 6: %.2fV".format(cellSix))
cellSixString.setSpan(StyleSpan(Typeface.BOLD), 0, 6, 0)
val cellSevenString = SpannableString("Cell 7: %.2fV".format(cellSeven))
cellSevenString.setSpan(StyleSpan(Typeface.BOLD), 0, 6, 0)
val cellEightString = SpannableString("Cell 8: %.2fV".format(cellEight))
cellEightString.setSpan(StyleSpan(Typeface.BOLD), 0, 6, 0)
val cellNineString = SpannableString("Cell 9: %.2fV".format(cellNine))
cellNineString.setSpan(StyleSpan(Typeface.BOLD), 0, 6, 0)
val cellTenString = SpannableString("Cell 10: %.2fV".format(cellTen))
cellTenString.setSpan(StyleSpan(Typeface.BOLD), 0, 7, 0)
val cellElevenString = SpannableString("Cell 11: %.2fV".format(cellEleven))
cellElevenString.setSpan(StyleSpan(Typeface.BOLD), 0, 7, 0)
val cellTwelveString = SpannableString("Cell 12: %.2fV".format(cellTwelve))
cellTwelveString.setSpan(StyleSpan(Typeface.BOLD), 0, 7, 0)
val cellThirteenString = SpannableString("Cell 13: %.2fV".format(cellThirteen))
cellThirteenString.setSpan(StyleSpan(Typeface.BOLD), 0, 7, 0)
val cellFourteenString = SpannableString("Cell 14: %.2fV".format(cellFourteen))
cellFourteenString.setSpan(StyleSpan(Typeface.BOLD), 0, 7, 0)
val packTotalString = SpannableString("Pack Total: %.1fV".format(totalVoltage))
packTotalString.setSpan(StyleSpan(Typeface.BOLD), 0, 10, 0)
val socString = SpannableString("SOC: ${data[17].toInt()}%")
socString.setSpan(StyleSpan(Typeface.BOLD), 0, 3, 0)
val chargeTempString = SpannableString("Charge Temp: ${chargeTempFahr.toInt()}" "°F")
chargeTempString.setSpan(StyleSpan(Typeface.BOLD), 0, 11, 0)
val dischargeTempString = SpannableString("Discharge Temp: ${dischargeTempFahr.toInt()}" "°F")
dischargeTempString.setSpan(StyleSpan(Typeface.BOLD), 0, 15, 0)
val chargeCurrentString = SpannableString("Charge Current: $chargeCurrent" "A")
chargeCurrentString.setSpan(StyleSpan(Typeface.BOLD), 0, 14, 0)
// val dischargeCurrentString = "Discharge Current: $dischargeCurrent" "A"
val chargeCircuitStateString = SpannableString("Charge Circuit: $chargeCircuit")
chargeCircuitStateString.setSpan(StyleSpan(Typeface.BOLD), 0, 14, 0)
val dischargeCircuitStateString = SpannableString("Discharge Circuit: $dischargeCircuit")
dischargeCircuitStateString.setSpan(StyleSpan(Typeface.BOLD), 0, 17, 0)
val balanceCircuitStateString = SpannableString("Balance Circuit: $balanceCircuit")
balanceCircuitStateString.setSpan(StyleSpan(Typeface.BOLD), 0, 15, 0)
val emptyCircuitStateString = SpannableString("Empty Circuit: $emptyCircuit")
emptyCircuitStateString.setSpan(StyleSpan(Typeface.BOLD), 0, 13, 0)
bmsVersionView.text = bmsVersionString
boardVersionView.text = boardVersionString
cellOneView.text = cellOneString
cellTwoView.text = cellTwoString
cellThreeView.text = cellThreeString
cellFourView.text = cellFourString
cellFiveView.text = cellFiveString
cellSixView.text = cellSixString
cellSevenView.text = cellSevenString
cellEightView.text = cellEightString
cellNineView.text = cellNineString
cellTenView.text = cellTenString
cellElevenView.text = cellElevenString
cellTwelveView.text = cellTwelveString
cellThirteenView.text = cellThirteenString
cellFourteenView.text = cellFourteenString
packTotalView.text = packTotalString
packSocView.text = socString
chargeTempView.text = chargeTempString
dischargeTempView.text = dischargeTempString
chargeCurrentView.text = chargeCurrentString
// dischargeCurrentView.text = dischargeCurrentString
chargeCircuitStateView.text = chargeCircuitStateString
dischargeCircuitStateView.text = dischargeCircuitStateString
balanceCircuitStateView.text = balanceCircuitStateString
emptyCircuitStateView.text = emptyCircuitStateString
}
Can I keep it as is or would it be better to seperate it out for readability?
CodePudding user response：
When you find yourself copy-pasting code, always stop and consider whether you can solve the same task by iterating or creating a function.
You are doing the same thing to every String, making the text up to the : bold, so you can write a function for that and eliminate a bunch of code:
private fun String.withStyling() = SpannableString(this).apply {
setSpan(StyleSpan(Typeface.BOLD), 0, indexOfFirst(':'), 0)
}
// and then for example:
bmsVersionView.text = "BMS Version: $bmsVersion".withStyling()
boardVersionView.text = "Board Version: 2.1".withStyling()
cellOneView.text = "Cell 1: %.2fV".format(cellOne).withStyling()
// ...
Also, you are doing the exact same thing with every cell view. So make a list of them and iterate!
val cellViews = listOf(
cellOneView,
cellTwoView,
//...
)
for ((i, cellView) in cellViews.withIndex()) {
val value = data[2 i].toDouble() / 100 3.52
val cellNumberString = (i 1).toString()
cellValue.text = "Cell $cellNumberString: %.2fV".format(value).withStyling()
}
CodePudding user response：
First of all, I will strongly suggest using view & data binding. It will reduce the findviewById lines; it's 2022, and you must change this behavior.
Secondly, separate the code into multiple functions. Otherwise, it will produce a code smell.