lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael McCandless (Commented) (JIRA)" <>
Subject [jira] [Commented] (LUCENE-3756) Don't allow IndexWriterConfig setters to chain
Date Fri, 10 Feb 2012 01:28:00 GMT


Michael McCandless commented on LUCENE-3756:

bq. Are you against StringBuilder too?

Actually I think that's a very appropriate use of builder.  The
chaining on one line bothers me less here, I think because it's nearly
always append, and it's nearly always short things being appended.  I
don't usually see it abused...

FST.Builder is another appropriate usage of builder (without chaining).

Other uses (eg for creating a ParallelReader), I don't think are
appropriate: it's an API change vs 3.x, it's extra work for the common
case, it creates API inconsistency (other IRs don't require builders).

bq. I seriously doubt that we have A coding style, much less one that WE ALL follow.

In fact we do have a standard coding style (it's referenced from
HowToContribute on the wiki).

It's true, we don't uniformly/aggressively enforce it.  We can
consider doing so... it's neat you get a failure from ant test if you
break the style ;)  But I have mixed feelings on whether we should...

What's important is we do have a clear standard, so if there is any
doubt, disagreement, etc., it's quickly settled, instead of huge
discussions (like this!): there is no (little) ambiguity; we all can
fix the code style as we work in a given area.

Eg, when I see a single line if statement missing the { }'s, I add

bq. You can even write a validator that ensures IWC is not used as a Builder pattern in Lucene


I don't like the ambiguity and easy "abuse" made possible by
chaining.  If we adopt additions to our coding style (Sun's guidelines
don't seem to cover chained methods ;) )... then I think this will
address my concerns.

My proposal would be (Robert als suggested that to me privately):
 * Allow definition of builder patterns, but dont force users to use them
 * Don't use the builders to chain in tests (unless it makes it more readable - it does quite
often), at least format the tests like I did in 3736.

+1, I think this is a good compromise.  The builder is optional (ie
you can invoke a big-ctor instead), and we have clear code style when
builders are invoked in Lucene.

bq. This is indicative of a larger worrying trend of people thinking of how to do something
the best way and then trying to prohibit all other ways. It ultimately makes Lucene less flexible.

Hmm can you give other examples of this trend?

> Don't allow IndexWriterConfig setters to chain
> ----------------------------------------------
>                 Key: LUCENE-3756
>                 URL:
>             Project: Lucene - Java
>          Issue Type: Improvement
>            Reporter: Michael McCandless
>            Assignee: Michael McCandless
> Spinoff from LUCENE-3736.
> I don't like that IndexWriterConfig's setters are chainable; it
> results in code in our tests like this:
> {noformat}
> IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig( TEST_VERSION_CURRENT,
new MockAnalyzer(random)).setMaxBufferedDocs(2).setMergePolicy(newLogMergePolicy()));
> {noformat}
> I think in general we should avoid chaining since it encourages hard
> to read code (code is already hard enough to read!).

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:!default.jspa
For more information on JIRA, see:


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

View raw message