lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shai Erera (JIRA)" <>
Subject [jira] [Commented] (SOLR-8475) Some refactoring to SolrIndexSearcher
Date Sat, 02 Jan 2016 19:52:39 GMT


Shai Erera commented on SOLR-8475:

bq. Once something is committed I just tend to leave the formatting alone unless I'm actively
editing that part of the code, or if it's really bad.

I usually follow that pattern too, however in this case I also re-formatted {{SolrIndexSearcher}}
code (thought it'd be nice). But also, if we don't change our standard settings, then with
the current settings, whenever I'll hit ENTER in a class, it will indent the empty line. Or,
if I write new code and want to use that builder-pattern, it will rewrap lines. So, I think
we should change those two settings that I included in the patch. I doubt that people really
want to see indented empty lines, or forcing re-wrapping of wrapped lines.

bq. it's usually only constants (i.e. static final strings/ints/empty_maps)

LOG is a static final field and I try to follow that convention with any static-final field.
Also, we already sometimes use "log" and other times "logger" and even with the casing we're
not consistent ({{LineRandomizedMapper}}, {{TreeMergeMapper}}, {{SafeConcurrentUpdateSolrClient}}
to name a few).

bq. Back compat breaks

True, but it's a really simple one to address ("Organize Imports" in any modern IDE). I can
add a note in CHANGES? Really, the class is huge and even though extracting those two classes
does not greatly reduce its size, it's a change in the right direction. And it's really a
simple break to address. What do you think?

> Some refactoring to SolrIndexSearcher
> -------------------------------------
>                 Key: SOLR-8475
>                 URL:
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 5.5, Trunk
>         Attachments: SOLR-8475.patch, SOLR-8475.patch, SOLR-8475.patch, SOLR-8475.patch
> While reviewing {{SolrIndexSearcher}}, I started to correct a thing here and there, and
eventually it led to these changes:
> * Moving {{QueryCommand}} and {{QueryResult}} to their own classes.
> * Moving FilterImpl into a private static class (was package-private and defined in the
same .java file, but separate class).
> * Some code formatting, imports organizing and minor log changes.
> * Removed fieldNames (handled the TODO in the code)
> * Got rid of usage of deprecated classes such as {{LegacyNumericUtils}} and {{Legacy-*-Field}}.
> I wish we'd cut down the size of this file much more (it's 2500 lines now), but I've
decided to stop here so that the patch is manageable. I would like to explore further refactorings
afterwards, e.g. extracting cache management code to an outer class (but keep {{SolrIndexSearcher}}'s
API the same, if possible).
> If you have additional ideas of more cleanups / simplifications, I'd be glad to do them.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message