Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/970
@ottobackwards You're absolutely not confusing things. It's important to
have that perspective, and I am 100% appreciative of you hopping in and
offering up your opinion. I think you're right that we probably need to reach
a solid compromise position rather than boiling the ocean.
@nickwallen What if, for this PR, we do the following:
1. Mostly keep the interface refactoring from the branch.
2. Instead of pushing everything directly into SolrUpdateMetaAlertDao and
ElasticsearchUpdateMetaAlertDao, we push to AbstractLuceneUpdateMetaAlertDao
minimally as needed to prevent duping (which is admittedly substantial)?
Everything else pushes down to the specific DAO which extends the abstract
DAOs. As in the test branch, the SolrMetaAlertDao just uses and passes along
the appropriate impls as needed.
3. Repeat (2) for the other DAOs implementations
4. I take another pass at tests to make sure both the common and specific
impls are properly tested.
At that point, it should somewhat clean up the type hierarchy which will
enable us to make things more pluggable in the future (including potentially
working to eliminate the aggregate marker interfaces). At the very least, I
think it makes it more straightforward, although it's not perfect. This should
improve maintainability and testing. The testing pass should make sure we're at
least better positioned to be confident in future refactorings.
I 100% admit this doesn't completely address the composition vs. extension,
but it at least makes progress relative towards that goal relative to where we
were. It has the benefit of being a doable fairly low-risk refactoring that
enables the PR to make it into the branch.
Hopefully this has the elements of a good compromise: Nobody is overjoyed,
but nobody feels cheated. In addition, we at least have set ourselves up to
make it easier for everyone to be happier in the future.
---
