-
Notifications
You must be signed in to change notification settings - Fork 38
Refactor measure scoring: Extract FHIR-agnostic logic and add score c… #841
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
Draft
lukedegruchy
wants to merge
3
commits into
master
Choose a base branch
from
ld-20251126-measure-scoring-fhir-version-agnostic
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Refactor measure scoring: Extract FHIR-agnostic logic and add score c… #841
lukedegruchy
wants to merge
3
commits into
master
from
ld-20251126-measure-scoring-fhir-version-agnostic
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…alculation infrastructure
Move scoring calculation logic from FHIR-specific classes to version-agnostic classes
to improve code reusability across FHIR versions (R4, DSTU3, R5). This is the first
phase of a larger refactoring to centralize scoring logic.
Changes:
- Add score storage to Def classes:
* Add measureScore and measureScoreSet fields to GroupDef with getter/setter/has methods
* Convert StratumDef from record to class to support mutable score storage
* Add getCountForScoring(boolean) helper to PopulationDef for unified count retrieval
- Extract aggregate calculation logic to BaseMeasureReportScorer:
* Move aggregateDoubles() method supporting SUM, MAX, MIN, AVG, COUNT, MEDIAN
* Refactor R4MeasureReportScorer.aggregate() to use base class method
* R4 version now wraps/unwraps Quantity objects around base calculation
- Extract continuous variable calculation helpers:
* Add calculateContinuousVariableScore() to BaseMeasureReportScorer
* Version-agnostic method works with Collection<Double> instead of FHIR types
- Add comprehensive score calculation to MeasureEvaluator:
* Add calculateScores(MeasureDef) public method for post-evaluation scoring
* Implement scoring for proportion, ratio, and continuous variable measures
* Add stratifier scoring support
* Create helper methods: calculateGroupScore(), calculateStratumScore(),
calcProportionScore(), aggregateValues(), extractNumericValues()
- Change R4MeasureReportScorer.calculateContinuousVariableAggregateQuantity() from
static to instance method to support base class usage
- Add Claude Sonnet 4.5 attribution comments to all modified/new methods
- Apply Spotless code formatting to all modified files
Impact:
- All 908 unit tests passing (0 failures, 0 errors)
- BaseMeasureReportScorer increased by 74 lines of reusable logic
- MeasureEvaluator increased by 400 lines of version-agnostic scoring
- R4MeasureReportScorer reduced by 42 net lines through extraction
- No functional changes to measure evaluation or scoring behavior
- Foundation laid for future FHIR version implementations
- Future work: Integrate score calculation into evaluation flow and simplify
FHIR-specific scorers to read pre-calculated scores
Files modified:
- GroupDef.java (+13 lines)
- StratumDef.java (converted to class, +35 net lines)
- PopulationDef.java (+19 lines)
- BaseMeasureReportScorer.java (+74 lines)
- R4MeasureReportScorer.java (-42 net lines)
- MeasureEvaluator.java (+400 lines)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
|
Formatting check succeeded! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #841 +/- ##
============================================
- Coverage 73.69% 73.20% -0.50%
Complexity 277 277
============================================
Files 574 575 +1
Lines 26783 26961 +178
Branches 3405 3437 +32
============================================
- Hits 19737 19736 -1
- Misses 5383 5559 +176
- Partials 1663 1666 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…capsulation Follow-up cleanup to commit 316a8cb that improves code quality and maintainability of the measure scoring refactoring. This commit removes redundant code, simplifies boolean flag patterns to use @nullable, and improves encapsulation by moving the populationBasis responsibility into PopulationDef. Changes: 1. Simplify score storage pattern in GroupDef and StratumDef: - Remove redundant measureScoreSet boolean flags - Use @nullable pattern: hasMeasureScore() now checks measureScore != null - Cleaner, more idiomatic Java code with fewer fields to maintain 2. Improve PopulationDef encapsulation: - Add populationBasis CodeDef field to PopulationDef - Update both constructors (5-param and 7-param) to accept populationBasis - Add getPopulationBasis() getter method - Remove boolean parameter from getCountForScoring() - now uses internal field - Better encapsulation: PopulationDef knows its own basis instead of relying on caller 3. Remove code duplication in BaseMeasureReportScorer: - Delete aggregateDoubles() method (30 lines) - duplicated in MeasureEvaluator - Delete calculateContinuousVariableScore() method (7 lines) - wrapper around above - R4MeasureReportScorer.aggregate() now self-contained with inline logic - Reduces maintenance burden and potential for divergence 4. Update all call sites: - R4MeasureDefBuilder: Pass populationBasisDef to PopulationDef constructors (2 sites) - Dstu3MeasureDefBuilder: Pass populationBasisDef to PopulationDef constructor (1 site) - MeasureEvaluator.getPopulationCount(): Remove boolean parameter from getCountForScoring() - Test files: Update PopulationDef constructor calls and add missing imports (3 test files) Impact: - All 908+ unit tests passing (0 failures, 0 errors) - Code formatted with Spotless (3 files cleaned) - Net reduction of ~44 lines of code - Improved code clarity and maintainability - No functional changes to measure evaluation or scoring behavior Files modified: - GroupDef.java: Removed measureScoreSet field, simplified hasMeasureScore() (-2 lines) - StratumDef.java: Removed measureScoreSet field, simplified hasMeasureScore() (-2 lines) - PopulationDef.java: Added populationBasis field, updated constructors and getCountForScoring() (+8 lines) - BaseMeasureReportScorer.java: Removed duplicate aggregation methods (-74 lines) - R4MeasureReportScorer.java: Made aggregate() self-contained (+18 lines) - MeasureEvaluator.java: Removed parameter from getCountForScoring() call (-1 line) - R4MeasureDefBuilder.java: Pass populationBasis to constructors (+7 lines) - Dstu3MeasureDefBuilder.java: Pass populationBasis to constructor (+10 lines) - PopulationDefTest.java: Updated constructors, added import (+18 lines) - R4MeasureReportBuilderTest.java: Updated constructor (+2 lines) - R4PopulationBasisValidatorTest.java: Updated constructor (+2 lines) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
… organization and maintainability
Extract all measure scoring calculation logic from MeasureEvaluator into a new static
utility class MeasureEvaluatorScorer. This improves code organization by separating
concerns and makes the scoring logic more reusable and maintainable.
Changes:
1. Create new MeasureEvaluatorScorer utility class:
- Final class with private constructor to prevent instantiation
- 14 static methods extracted from MeasureEvaluator
- Package-private visibility for internal methods, public for calculateScores() entry point
- Methods: calculateScores(), calculateGroupScore(), calcProportionScore(),
getPopulationCount(), calculateContinuousVariableGroupScore(),
calculateRatioContVariableGroupScore(), calculateStratumScore(),
getStratumPopulationCount(), calculateContinuousVariableStratumScore(),
calculateRatioContVariableStratumScore(), findStratumPopulation(),
extractNumericValues(), extractNumericValuesFromStratumResources(),
aggregateValues()
2. Remove scoring methods from MeasureEvaluator:
- Deleted 14 scoring methods (392 lines) from MeasureEvaluator
- MeasureEvaluator now focuses solely on population evaluation
- Scoring logic ready to be integrated when needed (not currently called)
3. Add convenience method to PopulationDef:
- Add isBooleanBasis() method following StratumPopulationDef pattern
- Update getCountForScoring() to use isBooleanBasis() internally
- Improves code readability and reduces duplication
4. Enhance test coverage in PopulationDefTest:
- Add 10 new test methods for different population basis types
- Test with Encounter basis: setHandlingStringsWithEncounterBasis(),
setHandlingIntegersWithEncounterBasis(), setHandlingEncountersWithEncounterBasis()
- Test with date basis: setHandlingStringsWithDateBasis(),
setHandlingIntegersWithDateBasis(), setHandlingEncountersWithDateBasis()
- Test isBooleanBasis(): testIsBooleanBasisTrue(), testIsBooleanBasisFalseWithEncounter(),
testIsBooleanBasisFalseWithDate(), testGetCountForScoringWithBooleanBasis()
Impact:
- All 918 unit tests passing (0 failures, 0 errors, 13 skipped)
- Net reduction of 238 lines in MeasureEvaluator (394 removed, 156 test additions)
- MeasureEvaluatorScorer: 424 lines of reusable, version-agnostic scoring logic
- Improved separation of concerns: evaluation vs. scoring
- Foundation for future integration of scoring calculations
- No functional changes to measure evaluation behavior
Files modified:
- MeasureEvaluator.java: Removed 14 scoring methods (-392 lines)
- MeasureEvaluatorScorer.java: New class with extracted scoring logic (+424 lines)
- PopulationDef.java: Added isBooleanBasis() convenience method (+13 lines)
- PopulationDefTest.java: Added 10 new test methods (+143 lines)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…alculation infrastructure
Move scoring calculation logic from FHIR-specific classes to version-agnostic classes to improve code reusability across FHIR versions (R4, DSTU3, R5). This is the first phase of a larger refactoring to centralize scoring logic.
Changes:
Impact:
Files modified:
🤖 Generated with Claude Code