Erick Erickson created SOLR-14474:
-------------------------------------

             Summary: 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


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

Reply via email to