-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update decorated sensor state changes to look a little flux-ier #26
base: master
Are you sure you want to change the base?
Conversation
const newBracket = this.calculateAQINotifyBracket(data.AQI) || this.currentAQINotifyBracket; | ||
this.calculateAQIThresholds(data.AQI); | ||
if(newBracket && newBracket !== this.currentAQINotifyBracket) { | ||
this.currentAQINotifyBracket = this.calculateAQINotifyBracket(data.AQI); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the "magic" line that becomes way more explicit in the updateAQIMonitoringState method
Note this does not add any functionality except in more explicit logging, it's a strict refactor that keeps the tests green. |
return this.formatReport(data); | ||
} | ||
|
||
private updateAQIMonitoringState(AQI: number, forceReset = false) : { old: monitoringState; new: monitoringState, bracketChange: notifyBracket, changed: boolean } { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Old and new states are only used for logging at this point. The bracketChange and changed flags are the relevant ones. Bracket change is whether the AQI is outside of the previous AQI boundaries (and in what direction).
Not sure if I want to do this refactor or not. This makes all the state changes more explicit but also wordier. If it looks like I want to move this direction in the future I can build on it?