lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael McCandless (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LUCENE-3756) Don't allow IndexWriterConfig setters to chain
Date Thu, 09 Feb 2012 00:32:57 GMT

    [ https://issues.apache.org/jira/browse/LUCENE-3756?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204149#comment-13204149
] 

Michael McCandless commented on LUCENE-3756:
--------------------------------------------

bq. I can on the opposite also write all parts of a chaining api in separate lines and that's
what I generally do (look at my StringBuilders when I code something).

The problem is that chaining is easily (and, often, from what I've
seen) abused and quickly results in very large compound expressions
that are hard to read (to my eyes anyway).  Just grep for
newIndexWriterConfig in our tests to see many examples...

Readability is important.

bq. If you like, I will ask my Eclipse to rewrite all tests to make chains in new lines (if
it can do this, because it did this when coding the test).

+1

Is it easy to have Eclipse fully un-chain it?  Ie so instead of:

{noformat}
IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig( TEST_VERSION_CURRENT, new
MockAnalyzer(random)).setMaxBufferedDocs(2).setMergePolicy(newLogMergePolicy()));
{noformat}

We'd have something like this:

{noformat}
IndexWriterConfig iwc = newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random));
iwc.setMaxBufferedDocs(2);
iwc.setMergePolicy(newLogMergePolicy());

IndexWriter writer = new IndexWriter(dir, iwc);
{noformat}

?  That would be better... (I think).

bq. The point is that I have the freedom to decide how to use the class.

Not all freedom is good.  Java and C++ (but not Python!) let you
indent code however you want, thanks to { } defining scopes, but that
doesn't mean you should be taking advantage of that freedom by
indenting source code in unexpected ways.

And in fact we are not free to take advantage of that in Lucene
because (thankfully) we have a common coding style that we all follow.

Having this freedom suddenly means we have ambiguity on how we format
these chained expressions, and I don't like adding ambiguity in our
sources: there should [generally] be one clear way of doing something.
We have enough ambiguity as it is...

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.

                
> Don't allow IndexWriterConfig setters to chain
> ----------------------------------------------
>
>                 Key: LUCENE-3756
>                 URL: https://issues.apache.org/jira/browse/LUCENE-3756
>             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: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

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


Mime
View raw message