lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McCandless <luc...@mikemccandless.com>
Subject Re: Turning IndexReader.isDeleted implementations to final
Date Sun, 28 Feb 2010 18:29:51 GMT
Sorry, I think the classes?  Not sure which others should be...

Though it always spooks me out trying to decide if something should
really be final... these two are package private so in theory nobody
should be extending them, anyway, but if out there someone subclassed
them (mixing code into oal.index) to fix something and didn't post
back to us then we're slamming that door shut... all for likely tiny
(if at all) perf gains.  But then if we do make them final we would
then hear from any (if any) places that broke about what we should
fix/make extensible in our core impls..

I think using final for classes like any of our analyzers makes alot
of sense.  There we explicitly want to state that you should go make
your own class if you need a custom analyzer.  Too many problems
surface if you subclass core analyzers.

Also... making DirectoryReader final would prevent any sneaky
subclassing out there... which'd be good because I don't actually like
that DirectoryReader (and, MultiReader) even subclass IndexReader.  I
don't think they should, ie, I think it's bad that you can get
Multi*Enum, call isDeleted (doing a binary search each time).  You
should work per segment (use MultiSearcher) instead.  So tightening
these classes up for that possible future seems good?  Maybe we should
even deprecate them!

So +1 to make these 2 final, and maybe also MultiReader, but not sure
which others...

Mike

On Sun, Feb 28, 2010 at 12:10 PM, Shai Erera <serera@gmail.com> wrote:
> What's ok? making the classes final or just the method declaration? If
> classes, besides ReadOnlySegmentReader, which other impls do you think can
> be made final (I'm not in front of the code)?
>
> On Sun, Feb 28, 2010 at 7:05 PM, Michael McCandless
> <lucene@mikemccandless.com> wrote:
>>
>> Seems OK I think?
>>
>> Mike
>>
>> On Sun, Feb 28, 2010 at 12:37 AM, Shai Erera <serera@gmail.com> wrote:
>> > Hi
>> >
>> > Do you think it's worth to make some of the isDeleted method impls
>> > final,
>> > like in ReadOnlySegmentReader and (maybe) DirectoryReader? I'm thinking
>> > the
>> > classes that are perceived as final could benefit from that, since their
>> > impl could be inlined. Maybe just make these classes final entirely?
>> >
>> > Shai
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: java-dev-help@lucene.apache.org
>>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


Mime
View raw message