I have a codebase where all my data calculations and data formatting occur within a single function. Are there any performance benefits to separating these out into separate functions or does separating them only improve readability?
I know that I should separate them but I don't really know all the reasons why.
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
}
CodePudding user response:
Performance? No. Maintainability? Yes. Separation of concerns is one of the core tenets of modern software architecture. Combining the two things makes a difficult to read, difficult to debug mess. Separating them allows you to concentrate on one thing at a time, and makes it easier for people maintaining the code (which may even be you 6 months from now when fixing a bug and you've forgotten how it works) to understand the logic and flow of the program.
That function you posted would not be accepted in any professional codebase. It's too long, it does too many things. It needs to be broken up.