Refactor AggregatorFactory interface into two different interfaces, return the second one from getCombiningFactory()
- Dominant language
- Java
- Stars
- 14.1k
- Forks
- 3.8k
- Avg merge
- 2d 58m
- Merged PRs (30d)
- 233
Description
Currently, `AggregatorFactory.getCombiningFactory()` and `getMergingFactory()` should return an instance of `AggregatorFactory` interface, which has a lot of methods that are common with the base `AggregatorFactory`. It provokes developers to "cut the corner" by extending the base `AggregatorFactory`. But they often forget to override `makeAggregateCombiner()` method, see #6039 and #7243.
Solution: extract a separate aggregating interface that has only aggregation-related methods and methods that manipulate the objects, but doesn't have `getName()`, `requiredFields()` (?), `getTypeName()` (?), and `getMergingFactory()` and `getCombiningFactory()` themselves. Then this secondary interface is returned from methods `getCombiningFactory()` and `getAggregationFactory()` (a new method).
It may be that this secondary interface should better be called "AggregatorFactory" (like the former conflated interface), while the basic interface should be renamed somehow.
Other issues that might be related: #7019 and #6858.
Contributor guide
Research direction
Read the AggregatorFactory interface and trace getCombiningFactory(), getMergingFactory(), and the proposed getAggregationFactory() through their implementations. Review #6039, #7243, #7019, and #6858 for context; done means the aggregation-only interface and return types are settled without requiring unrelated methods.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100