lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shai Erera (Commented) (JIRA)" <>
Subject [jira] [Commented] (LUCENE-3756) Don't allow IndexWriterConfig setters to chain
Date Thu, 09 Feb 2012 04:24:01 GMT


Shai Erera commented on LUCENE-3756:

So, I don't think we should be adding any more builders/chaining to
Lucene's sources... Apps can always create builders on top of Lucene.

That's a bit extreme I think. The API should be clear, and easy to consume/use. Why force
an app to wrap and object with a Builder if we can do it in Lucene? What else do you want
to return from all the setters in IWC? They return void, so why not create the flexibility
to the users?

Are you against StringBuilder too?

bq. because (thankfully) we have a common coding style that we all follow

I seriously doubt that we have A coding style, much less one that WE ALL follow. Look at Lucene
code -- a lot of it is badly formatted. Some people like to write their if statements without
{} (if they contain a single instruction), others prefer to always put the braces. Some don't
like redundant if-else statements if both the 'if' and 'else' return from the method (Eclipse
even has an option to show a warning on it), others prefer to always put an if-else for readability.

Coding style is important, I agree. In my project, we enforce coding style through unit tests
- we wrote a framework which checks the source code and asserts all sorts of things like line
length (we limit them to 120 chars), spaces before/after opening braces, copyright notices,
indentation and what not. We even have a validator that ensures you don't put 2 consecutive
empty lines in the code :). The framework is executed as part of 'ant test', so these things
are always checked, and unless you don't execute tests before commit, there's no chance to
commit code which doesn't follow the coding style that we've decided upon. I thought about
contributing it to Lucene, but wasn't sure if the community cares much about code formatting/style.

If we had such a framework, you could easily write a validator which enforces a max-length
on a line (we already do), so you wouldn't end up with such long lines in the first place.
You can even write a validator that ensures IWC is not used as a Builder pattern in Lucene
code. The great thing about this framework is that it doesn't force you to change the API
signature. You can still keep IWC as a Builder, but enforce that it's not used in Lucene like
that, and I can still enjoy IWC as I do today, chaining calls when I feel it's appropriate.

Mike - if you want to change how IWC is used in Lucene tests, let's do that. But I prefer
that we keep the builder API and not enforce coding style on the users.
> 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