Skip to content
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

Convert Nightscout command to coroutines #202

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Conversation

p5nbTgip0r
Copy link
Member

This is the biggest command Diabot has (by line count), so I felt it would benefit the most from converting to coroutines first. Using coroutines allows the code to look similar to standard blocking code, and typically results in better readability.

Some parts of this could probably be improved more, but would require converting our existing frameworks (database I/O and Nightscout API) to coroutines.

This PR technically flags detekt, but these rules were already being broken with the original code. I tried to refactor it as much as I could to avoid triggering it, but I wasn't sure how to resolve the nesting one in getUnstoredData() cleanly. The nested block depth is the same as the old code as far as I could tell, but it wasn't being flagged for some reason.

@p5nbTgip0r p5nbTgip0r requested a review from cascer1 as a code owner November 1, 2023 01:59
@@ -109,7 +107,7 @@
* @param event Command event which called this command
* @return A nightscout DTO and an embed based on it
*/
private fun getUnstoredData(event: CommandEvent): Mono<Tuple2<NightscoutDTO, MessageEmbed>> {
private suspend fun getUnstoredData(event: CommandEvent): NightscoutUserDTO {

Check warning

Code scanning / detekt

Excessive nesting leads to hidden complexity. Prefer extracting code to make it easier to understand. Warning

Function getUnstoredData is nested too deeply.
@@ -177,71 +171,72 @@
* @param event Command event which called this command
* @return NS user DTO for this domain. This will be a generic DTO if there was no data found.
*/
private fun getDataFromDomain(domain: String, event: CommandEvent): Mono<NightscoutUserDTO> {
val userDtos = getUsersForDomain(domain)
private suspend fun getDataFromDomain(domain: String, event: CommandEvent): NightscoutUserDTO {

Check warning

Code scanning / detekt

Restrict the number of return statements in methods. Warning

Function getDataFromDomain has 3 return statements which exceeds the limit of 2.
Copy link
Member

@cascer1 cascer1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I've made some follow-up tickets to look into the Detekt alerts so we can improve the code.

@cascer1 cascer1 merged commit 613ec5b into main Nov 2, 2023
5 checks passed
@cascer1 cascer1 deleted the feat/nightscout-coroutines branch November 2, 2023 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants