-
Notifications
You must be signed in to change notification settings - Fork 0
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
Enforce valid KpiResult scores #30
Conversation
…they are in the range of 0...100
|
Code Coverage
Files
|
@@ -60,6 +61,8 @@ internal interface KpiCalculationStrategy { | |||
fun isValid(node: KpiNode, strict: Boolean = false): 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.
Is this intented to be on global scope? How does the private modifier work in that case?
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.
the logging library says to use it this way, I didn't check why and trusted them.
private makes it accessible in the whole file and it should be initialized first.
} | ||
} | ||
|
||
private fun <T : KpiCalculationResult> validateScore( |
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.
the method not only validates but also coerces
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.
fair point, will rename
@@ -149,4 +152,38 @@ internal abstract class BaseKpiCalculationStrategy : KpiCalculationStrategy { | |||
} | |||
|
|||
protected abstract fun internalIsValid(node: KpiNode, strict: Boolean): Boolean | |||
|
|||
companion object { | |||
fun getResultInValidRange(result: KpiCalculationResult): KpiCalculationResult { |
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.
is this a point where we should throw in strict mode?
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.
in strict mode we would fail even earlier as we then enforce the validity of the given KPI hierarchy and a valid hierarchy should not result in invalid values.
This function should mainly act as a failsafe if we somehow messed up and somewhere get invalid range results.
feat: check the score of all KpiCalculationResults to guarantee that they are in the range of 0...100