Function for formatting data retrieved from a battery via Bluetooth Low Energy

Question

I have the following function in Swift and am looking to improve the readability. The function takes from the deviceData object which holds maps of data like [String:Double], [String:Int], and sometimes just regular String. The function is meant to format the data in the maps into Strings and return an array of Strings. Adding the variables to the Array below all of the let variables seems kind of funky to me and I was thinking maybe it would be good to declare an empty array above, get rid of the variable declarations and just add each value to the array but I'm not sure if that would be better or worse for performance. How can I improve the readability of this function without sacrificing performance?

func createFormattedDataStrings(deviceData: DeviceData, timeMilli: Double) -> Array<String?> {
 
 let formattedBmsVersion = (deviceData.bmsVersion?.first!.key)! + " \(deviceData.bmsVersion!.first!.value)"
 let formattedBoardVersion = (deviceData.boardVersion?.first!.key)! + " \(deviceData.boardVersion!.first!.value)"
 let formattedCellOne = (deviceData.cellOne?.first?.key)! + String(format: "%.2f", deviceData.cellOne!.first!.value) + "V"
 let formattedCellTwo = (deviceData.cellTwo?.first!.key)! + String(format: "%.2f", deviceData.cellTwo!.first!.value) + "V"
 let formattedCellThree = (deviceData.cellThree?.first!.key)! + String(format: "%.2f", deviceData.cellThree!.first!.value) + "V"
 let formattedCellFour = (deviceData.cellFour?.first!.key)! + String(format: "%.2f", deviceData.cellFour!.first!.value) + "V"
 let formattedCellFive = (deviceData.cellFive?.first!.key)! + String(format: "%.2f", deviceData.cellFive!.first!.value) + "V"
 let formattedCellSix = (deviceData.cellSix?.first!.key)! + String(format: "%.2f", deviceData.cellSix!.first!.value) + "V"
 let formattedCellSeven = (deviceData.cellSeven?.first!.key)! + String(format: "%.2f", deviceData.cellSeven!.first!.value) + "V"
 let formattedCellEight = (deviceData.cellEight?.first!.key)! + String(format: "%.2f", deviceData.cellEight!.first!.value) + "V"
 let formattedCellNine = (deviceData.cellNine?.first!.key)! + String(format: "%.2f", deviceData.cellNine!.first!.value) + "V"
 let formattedCellTen = (deviceData.cellTen?.first!.key)! + String(format: "%.2f", deviceData.cellTen!.first!.value) + "V"
 let formattedCellEleven = (deviceData.cellEleven?.first!.key)! + String(format: "%.2f", deviceData.cellEleven!.first!.value) + "V"
 let formattedCellTwelve = (deviceData.cellTwelve?.first!.key)! + String(format: "%.2f", deviceData.cellTwelve!.first!.value) + "V"
 let formattedCellThirteen = (deviceData.cellThirteen?.first!.key)! + String(format: "%.2f", deviceData.cellThirteen!.first!.value) + "V"
 let formattedCellFourteen = (deviceData.cellFourteen?.first!.key)! + String(format: "%.2f", deviceData.cellFourteen!.first!.value) + "V"
 let formattedPackVoltage = (deviceData.packVoltage?.first!.key)! + String(format: "%.1f", deviceData.packVoltage!.first!.value) + "V"
 let formattedPackSoc = (deviceData.packSoc?.first!.key)! + String(deviceData.packSoc!.first!.value) + "%"
 let formattedChargeTemp = (deviceData.chargeTemp?.first!.key)! + " \(deviceData.chargeTemp!.first!.value)°F"
 let formattedDischargeTemp = (deviceData.dischargeTemp?.first!.key)! + " \(deviceData.dischargeTemp!.first!.value)°F"
 let formattedChargeCurrent = (deviceData.chargeCurrent?.first!.key)! + " \(deviceData.chargeCurrent!.first!.value)A"
 let formattedChargeCircuitState = (deviceData.chargeCircuitState?.first!.key)! + " \(deviceData.chargeCircuitState!.first!.value)"
 let formattedDischargeCircuitState = (deviceData.dischargeCircuitState?.first!.key)! + " \(deviceData.dischargeCircuitState!.first!.value)"
 let formattedBalanceCircuitState = (deviceData.balanceCircuitState?.first!.key)! + " \(deviceData.balanceCircuitState!.first!.value)"
 let formattedEmptyCircuitState = (deviceData.emptyCircuitState?.first!.key)! + " \(deviceData.emptyCircuitState!.first!.value)"
 let formattedRestMode = deviceData.restMode
 let formattedMisbalancedMode = deviceData.misbalancedMode
 let formattedLeftChargedMode = deviceData.leftChargedMode
 let formattedHighVoltageMode = deviceData.highVoltageMode
 let formattedHighTempMode = deviceData.highTempMode
 let formattedLowTempMode = deviceData.lowTempMode
 let formattedLowVoltageMode = deviceData.lowVoltageMode
 let formattedHibernationMode = deviceData.hibernationMode
 let formattedOverDischargeMode = deviceData.overDischargeMode
 let formattedShipMode = deviceData.shipMode
 let formattedStateError = deviceData.stateError
 let formattedFinalCharge = deviceData.finalCharge
 let formattedFinalDischarge = deviceData.finalDischarge
 let formattedFinalBalance = deviceData.finalBalance
 let formattedFinalEmpty = deviceData.finalEmpty
 
 
 var dataStringArray: Array<String?> = [formattedBmsVersion, formattedBoardVersion, formattedCellOne, formattedCellTwo, formattedCellThree, formattedCellFour, formattedCellFive, formattedCellSix, formattedCellSeven, formattedCellEight, formattedCellNine, formattedCellTen, formattedCellEleven, formattedCellTwelve, formattedCellThirteen, formattedCellFourteen, formattedPackVoltage, formattedPackSoc, formattedChargeTemp, formattedDischargeTemp, formattedChargeCurrent, formattedChargeCircuitState, formattedDischargeCircuitState, formattedBalanceCircuitState, formattedEmptyCircuitState, formattedRestMode, formattedMisbalancedMode, formattedLeftChargedMode, formattedHighVoltageMode, formattedHighTempMode, formattedLowTempMode, formattedLowVoltageMode, formattedHibernationMode, formattedOverDischargeMode, formattedShipMode, formattedStateError, formattedFinalCharge, formattedFinalDischarge, formattedFinalBalance, formattedFinalEmpty]
 
 if (deviceData.bmsVersion?.first?.value)! >= 3634 {
 let formattedHotChargerMode = deviceData.hotChargerMode
 let formattedBadCharger = deviceData.badCharger
 dataStringArray.insert(formattedHotChargerMode!, at: 30)
 dataStringArray.insert(formattedBadCharger!, at: 41)
 }
 
 return dataStringArray
}

Here is the deviceData specification:

import Foundation
public struct DeviceData {
 var bmsVersion: [String:Int]?
 var boardVersion: [String:Double]?
 var cellOne: [String:Double]?
 var cellTwo: [String:Double]?
 var cellThree: [String:Double]?
 var cellFour: [String:Double]?
 var cellFive: [String:Double]?
 var cellSix: [String:Double]?
 var cellSeven: [String:Double]?
 var cellEight: [String:Double]?
 var cellNine: [String:Double]?
 var cellTen: [String:Double]?
 var cellEleven: [String:Double]?
 var cellTwelve: [String:Double]?
 var cellThirteen: [String:Double]?
 var cellFourteen: [String:Double]?
 var packVoltage: [String:Double]?
 var packSoc: [String:Int]?
 var chargeTemp: [String:Int]?
 var dischargeTemp: [String:Int]?
 var chargeCurrent: [String:Int]?
 var chargeCircuitState: [String:String]?
 var dischargeCircuitState: [String:String]?
 var balanceCircuitState: [String:String]?
 var emptyCircuitState: [String:String]?
 var restMode: String?
 var misbalancedMode: String?
 var leftChargedMode: String?
 var highVoltageMode: String?
 var highTempMode: String?
 var hotChargerMode: String?
 var lowTempMode: String?
 var lowVoltageMode: String?
 var hibernationMode: String?
 var overDischargeMode: String?
 var shipMode: String?
 var stateError: String?
 var finalCharge: String?
 var finalDischarge: String?
 var finalBalance: String?
 var finalEmpty: String?
 var badCharger: String?
 var dischargeShort: String?
 
 func getDeviceDataCount() -> Int {
 if (hotChargerMode == nil) {
 return 40
 } else {
 return 43
 }
 }
}
```

• Martin R: Can you add the exact definition of DeviceData? Is there some specification of the data?
• Dylon Jaynes: @MartinR I just added it.
• Martin R: I just wonder why all the members are dictionaries, and only one entry of each dictionary is used. Is that intentional?
• Dylon Jaynes: @MartinR Yes, I intended to do that to accomplish what felt like easier data formatting for the display. I have data that I wanted to access and manipulate but I also had strings to describe that data so I just put them together with a data structure I was familiar with. Should I have used a different data structure?
• Dylon Jaynes: Also, both members of the dictionary are being used in the formatting strings.

Answer 1

A commenter has asked me to actually review the code. I expect that comparing the OP's code to my solution provides its own critique, but sure I can get more explicit...

A review of the type itself:

• Using a dictionary to represent a single String Value pair is problematic. Better to use a Tuple or a struct.
• Having a number of properties that are manually numbered (e.g., cellOne, cellTwo, etc.) creates unnecessary duplication. Just use an array.
• Using optional containers, String? in this case, is only valuable if there is a business case to be made between "empty" and "non-existent". I'm assuming there is for the context of this review, but I think it should be mentioned anyway.

A review of the function:

• There is lot's of duplication here that can be simplified. Specifically there are a limited number of ways to format the data.
• Moving this function into an extension of its first argument will dramatically limit the amount of boiler-plate code needed (the constant repetition of deviceData inside the function). Type extensions are a good way to establish scope when you are doing extensive work on a particular object/value.
• A let assignment is great for when the right side of the operator is much more complex than the left side, but not as valuable in other cases. For example, let formattedRestMode = deviceData.restMode doesn't really add value... Just use deviceData.restMode directly.

---

Here's the simplification I came up with. In the future, I suggest you write a unit test so that you can mess with the guts of the function and know that your changes still work...

func formatted<T>(_ value: (String, T)?, format: (T) -> String = { "\(0ドル)" }) -> String? {
 "\(value!.0) \(format(value!.1))"
}
extension DeviceData {
 func createFormattedDataStrings(timeMilli _: Double) -> [String?] {
 [
 formatted(bmsVersion),
 formatted(boardVersion)
 ] +
 cells.map { formatted(0,ドル format: { "\(String(format: "%.2f", 0ドル))V" }) } +
 [
 formatted(packVoltage, format: { "\(String(format: "%.1f", 0ドル))V" }),
 formatted(packSoc, format: { "\(0ドル)%" }),
 formatted(chargeTemp, format: { "\(0ドル)°F" }),
 formatted(dischargeTemp, format: { "\(0ドル)°F" }),
 formatted(chargeCurrent, format: { "\(0ドル)A" }),
 formatted(chargeCircuitState),
 formatted(dischargeCircuitState),
 formatted(balanceCircuitState),
 formatted(emptyCircuitState),
 restMode,
 misbalancedMode,
 leftChargedMode,
 highVoltageMode,
 bmsVersion!.1 >= 3634 ? hotChargerMode : nil,
 highTempMode,
 lowTempMode,
 lowVoltageMode,
 hibernationMode,
 overDischargeMode,
 shipMode,
 stateError,
 finalCharge,
 finalDischarge,
 finalBalance,
 finalEmpty,
 bmsVersion!.1 >= 3634 ? badCharger : nil
 ]
 }
}
public struct DeviceData {
 var bmsVersion: (String, Int)?
 var boardVersion: (String, Double)?
 var cells: [(String, Double)] = []
 var packVoltage: (String, Double)?
 var packSoc: (String, Int)?
 var chargeTemp: (String, Int)?
 var dischargeTemp: (String, Int)?
 var chargeCurrent: (String, Int)?
 var chargeCircuitState: (String, String)?
 var dischargeCircuitState: (String, String)?
 var balanceCircuitState: (String, String)?
 var emptyCircuitState: (String, String)?
 var restMode: String?
 var misbalancedMode: String?
 var leftChargedMode: String?
 var highVoltageMode: String?
 var highTempMode: String?
 var hotChargerMode: String?
 var lowTempMode: String?
 var lowVoltageMode: String?
 var hibernationMode: String?
 var overDischargeMode: String?
 var shipMode: String?
 var stateError: String?
 var finalCharge: String?
 var finalDischarge: String?
 var finalBalance: String?
 var finalEmpty: String?
 var badCharger: String?
 var dischargeShort: String?
}