Conversation
steve-curran
left a comment
steve-curran
left a comment
There was a problem hiding this comment.
Looks good. Let me know when it's ready for full review.
I've never written kotlin but I have written scala once the appropriate tests have been added. This looks similar, though I'm sure some of the nuances are lost on me.
| * Formats a message about the total flag count in the system based on the [teamFlags] | ||
| */ | ||
| private fun formatTotalCountMessage(totalFlagCount: Int): String { | ||
| private fun formatTotalCountMessage(teamFlags: List<FlagDetail>): String { |
There was a problem hiding this comment.
minor: If we follow the principle of least knowledge here and pass flagSize and medianAgeInDays as arguments, this function may be easier to test as we then do not need to mock/instantiate the teamFlags object to pass in.
|
|
||
| private const val MILLISECONDS_IN_A_DAY = 1000 * 60 * 60 * 24 | ||
| fun List<FlagDetail>.getMedianAgeInDays(): Long = this.sortedBy { it.creationDate }.let { flagDetails -> | ||
| if (flagDetails.size % 2 == 0) { |
There was a problem hiding this comment.
Excuse my lack of kotlin knowledge here.
Is it possible to extract flagDetails.size / 2 like statements to meaningful variables like medianFlag here to make this code easier to digest?
Can we utilize a guard clause to reduce the cognitive complexity of this block?
2 participants
Description / Purpose
Adding a calculation of the median age of a project and teams' flags into the Slack messages
Types of changes
What types of changes does your code ?
Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Todos
Screenshots
Questions / Notes
Learning / Resources