[jira] [Commented] (SOLR-14474) Fix remaining auxilliary class warnings in Solr

["Erick Erickson (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-14474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17114918#comment-17114918 ] 

Erick Erickson commented on SOLR-14474:
---------------------------------------

See the PR, I'll commit Tuesday unless there are objections.

> Fix remaining auxilliary class warnings in Solr
> -----------------------------------------------
>
>                 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
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> 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-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org