[ 
https://issues.apache.org/jira/browse/LUCENE-5387?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Shai Erera updated LUCENE-5387:
-------------------------------

    Attachment: LUCENE-5387.patch

Patch fixes FacetConfig.build to take and return Document. I had to modify it 
to take Document as well because if we take IndexDocument, we cannot add the 
Indexable/StorableFields to Document, as it only takes Field. I don't think it 
is critical that we make FacetConfig that generic - up until now users had to 
use FacetFields.addFields and pass Document, so we didn't make anything worse.

I didn't add CHANGES as this whole change wasn't released yet. However I wonder 
if we should mention somewhere that when users port their code to the new API, 
they don't forget to add the Document that was returned by FacetConfig.build(), 
and not the one that they passed in. I added warnings in the javadocs, but 
still...

> Improve FacetConfig.build
> -------------------------
>
>                 Key: LUCENE-5387
>                 URL: https://issues.apache.org/jira/browse/LUCENE-5387
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: modules/facet
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>         Attachments: LUCENE-5387.patch
>
>
> FacetConfig.build() takes an IndexDocument and returns a new instance of 
> IndexDocument. This forces you to write code like this:
> {code}
> Document doc = new Document();
> doc.add(new StringField("id", "someID", Store.NO));
> doc.add(new FacetField("author", "john"));
> IndexDocument facetDoc = facetConfig.build(doc);
> indexWriter.addDocument(facetDoc);
> {code}
> Technically, you don't need to declare 'facetDoc', you could just 
> {{indexWriter.addDocument(facetConfig.build(doc))}}, but it's weird:
> * After you call facetConfig.build(), you cannot add any more fields to the 
> document (since you get an IndexDoc), so you must call it last.
> * Nothing suggests you *should* call facetConfig.build() at all - I can 
> already see users trapped by the new API, thinking that adding a FacetField 
> is enough. We should at least document on FacetField that you should call 
> FacetConfig.build().
> * Nothing suggests that you shouldn't ignore the returned IndexDoc from 
> FC.build() - we should at least document that.
> I think that if FacetConfig.build() took an IndexDocument but returned a 
> Document, that will at least allow you to call it in whatever stage of the 
> pipeline that you want (after adding all FacetFields though)...
> I'll post a patch later.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

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

Reply via email to