lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McCandless <luc...@mikemccandless.com>
Subject Re: Lucene 2.9 and deprecated IR.open() methods
Date Sun, 04 Oct 2009 09:53:14 GMT
I don't think we should do both.  Suddenly, all code snippets (in
javadocs, tutorials, email we all send, etc.) can be one pattern or
the other, with each of us choosing based on our preference.  Or,
mixed.

I think this just causes confusion.  It'd suddenly become alot like
differences of opinion on which whitespace style is best.

I'd rather have one clear syntax, and at this point I'd prefer to
stick with the "classical" setter approach, ie a standalone config
object.

But there are two other (more important than pure syntax!) questions
being debated here:

  1 Do we prevent config settings from changing after creating an
    IW/IR?

  2 Do we use factory or ctor to create IW/IR?

On #1, we are technically taking something away.  Are we sure no users
find the freedom to change IW settings mid-stream (ramBufferSizeMB,
mergeFactor) important?  For example, infoStream should remain an IW
setter.  Also, MergePolicy now requires IW instance on construction,
so we'd need to rework that.

On #2, I agree with Michael: until we see a clear reason to hide IW's
concrete impl., we may as well stick with the one impl we have now.
Design for today.

Mike

On Sun, Oct 4, 2009 at 5:33 AM, Uwe Schindler <uwe@thetaphi.de> wrote:
>> >> The builder pattern and the config argument to a factory both have the
>> >> advantage that you can limit changes after creating an object.  Some
>> >> things are just bad to change in mid-stream.  The config argument is
>> >> nice in that you can pass it around to different stake holders, but
>> >> the builder can be used a bit like that as well.
>> >>
>> > Yeah that argument has been made. But I've *never* seen it as an issue.
>> > Just seems like a solution looking for a problem. I can see how it's
>> > cleaner, not missing that point. But still doesn't make me like it much.
>> >
>> >
>> Yeah personally this wasn't a problem for me either. I do like the
>> cleanliness though. Also, I'd very much prefer a config object over
>> multiple constructors (with the need to deprecate/add with every
>> change), as I proposed originally in this thread.
>>
>> I still don't see an advantage of the builder pattern over the config
>> object + factory pattern - and I'm not even sure if we really need a
>> factory; IMO passing a config object into a single constructor would be
>> sufficient for IW.
>
> For IR the factory would be ok. In my opinion you could also combine both
> patterns:
>
> - Each setter in the config object returns itself, so you have the builder
> pattern, but you could also use it in classical setter way (this only works
> if the builder pattern always returns itself not a new builder object)
> - The builder factory .build() just delegates to the ctor/static factory in
> IR/IW and passes itself to it).
>
> So you have both possibilities:
>
> IndexReader reader = new IndexReader.Config(dir).setReadOnly(true)
> .setTermInfosIndexDivisor(foo).build();
>
> is equal to:
>
> IndexReader.Config config = IndexReader.Config(dir);
> config.setReadOnly(true);
> config.setTermInfosIndexDivisor(foo);
> IndexReader reader = IndexReader.create(config);
>
> Uwe
>
>
> ---------------------------------------------------------------------
> 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