[
https://issues.apache.org/jira/browse/SOLR-14474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17105772#comment-17105772
]
Erick Erickson commented on SOLR-14474:
---------------------------------------
Thanks for the detailed response. Yeah, "zillion" is indefinite. If you take a
quick look at the PR I referenced, you'll see all the changes in the facet
code. That's going to be gnarly. I need to think a bit about how to make it a
smaller set of edits, what I did for SOLR-10810 was mindless. Helps to get a
sense of the problem though.
Your guidelines seem like a great place build from, that's certainly along the
lines of what I was thinking it's nice to see I wasn't totally off base.
> Fix auxilliary class warnings in Solr core
> ------------------------------------------
>
> Key: SOLR-14474
> URL: https://issues.apache.org/jira/browse/SOLR-14474
> Project: Solr
> Issue Type: Sub-task
> Reporter: Erick Erickson
> Assignee: Erick Erickson
> Priority: Major
>
> We have quite a number of situations where multiple classes are declared in a
> single source file, which is a poor practice. I ran across a bunch of these
> in solr/core, and [~mdrob] fixed some of these in SOLR-14426. [~dsmiley]
> looked at those and thought that it would have been better to just move a
> particular class to its own file. And [~uschindler] do you have any comments?
> I have a fork with a _bunch_ of changes to get warnings out that include
> moving more than a few classes into static inner classes, including the one
> Mike did. I do NOT intend to commit this, it's too big/sprawling, but it does
> serve to show a variety of situations. See:
> https://github.com/ErickErickson/lucene-solr/tree/jira/SOLR-10810 for how
> ugly it all looks. I intend to break this wodge down into smaller tasks and
> start over now that I have a clue as to the scope. And do ignore the generics
> changes as well as the consequences of upgrading apache commons CLI, those
> need to be their own JIRA.
> What I'd like to do is agree on some guidelines for when to move classes to
> their own file and when to move them to static inner classes.
> Some things I saw, reference the fork for the changes (again, I won't check
> that in).
> 1> DocValuesAcc has no fewer than 9 classes that could be moved inside the
> main class. But they all become "static abstract". And take
> "DoubleSortedNumericDVAcc" in that class, It gets extended over in 4 other
> files. How would all that get resolved? How many of them would people
> recommend moving into their own files? Do we want to proliferate all those?
> And so on with all the other plethora of classes in
> org.apache.solr.search.facet.
> This is particularly thorny because the choices would be about a zillion new
> classes or about a zillion edits.
> Does the idea of abstract .vs. concrete classes make any difference? IOW, if
> we change an abstract class to a nested class, then maybe we just have to
> change the class(es) that extend it?
> 2> StatsComponent.StatsInfo probably should be its own file?
> 3> FloatCmp, LongCmp, DoubleCmp all declare classes with "Comp" rather than
> "Cmp". Those files should just be renamed.
> 4> JSONResponseWriter. ???
> 5> FacetRangeProcessor seems like it needs its own class
> 6> FacetRequestSorted seems like it needs its own class
> 7> FacetModule
> So what I'd like going forward is to agree on some guidelines to resolve
> whether to move a class to its own file or make it nested (probably static).
> Not hard-and-fast rules, just something to cut down on the rework due to
> objections.
> And what about backporting to 8x? My suggestion is to backport what's
> easy/doesn't break back-compat in order to make keeping the two branches in
> sync.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
