Reset thresholds to default #426
Conversation
| [thresholdVeryLow, thresholdLow , thresholdMedium ,thresholdHigh, thresholdVeryHigh ] | ||
| } |
There was a problem hiding this comment.
[thresholdVeryLow, thresholdLow, thresholdMedium, thresholdHigh, thresholdVeryHigh]
Spaces 😄
There was a problem hiding this comment.
you're right, this looks AWFUL :D
AirCasting/Graph/GraphView.swift
Outdated
| .padding(.horizontal) | ||
| .padding(.top) |
There was a problem hiding this comment.
can we use .paddding([.horizontal, .top]) please?
| thresholdSettingsViewModel.thresholdVeryLow = "\(Int(thresholdValues[0]))" | ||
| thresholdSettingsViewModel.thresholdLow = "\(Int(thresholdValues[1]))" | ||
| thresholdSettingsViewModel.thresholdMedium = "\(Int(thresholdValues[2]))" | ||
| thresholdSettingsViewModel.thresholdHigh = "\(Int(thresholdValues[3]))" | ||
| thresholdSettingsViewModel.thresholdVeryHigh = "\(Int(thresholdValues[4]))" |
There was a problem hiding this comment.
What would you say about sth like:
` .onAppear {
thresholdSettingsViewModel.thresholdVeryLow = string(using: thresholdValues[0])
thresholdSettingsViewModel.thresholdLow = string(using: thresholdValues[1])
thresholdSettingsViewModel.thresholdMedium = string(using: thresholdValues[2])
thresholdSettingsViewModel.thresholdHigh = string(using: thresholdValues[3])
thresholdSettingsViewModel.thresholdVeryHigh = string(using: thresholdValues[4])
}
}
func string(using thresholdValue: Float) -> String {
return String(Int(thresholdValue))
}`
Just to consider: if not, then go below 😇
There was a problem hiding this comment.
Sure! tbh I didn't give it much thought - just changed this code to use the view model.
| func resetToDefault() -> [Float] { | ||
| var newInitial: [Float] = [] | ||
| for value in initialThresholds { | ||
| let newValue = Float(value) | ||
| newInitial.append(newValue) | ||
| } | ||
| return newInitial | ||
| } |
There was a problem hiding this comment.
Suggestion time:
func resetToDefault() -> [Float] { var newInitial: [Float] = [] initialThresholds.forEach { value in newInitial.append(Float(value)) } return newInitial }
| @@ -49,6 +49,9 @@ public class MeasurementStreamEntity: NSManagedObject, Identifiable { | |||
| public var localID: MeasurementStreamLocalID { | |||
| MeasurementStreamLocalID(id: objectID) | |||
| } | |||
| public var thresholds: [Int32] { | |||
| [thresholdVeryLow, thresholdLow, thresholdMedium, thresholdHigh, thresholdVeryHigh] | |||
There was a problem hiding this comment.
Perhaps this could be the thresholds struct and not an array? Using array in this case seems prone to error and we already know there can be only this limited set of 5 thresholds, so using a struct (or enum or dictionary) seems like a good fit. If that changes in the future, for example we will have 4 thresholds, it will be easier to edit without errors caused by thresholds[4].
I renamed HeatmapSettingsView because I think this name would be confusing after implementing heatmap.