[ https://issues.apache.org/jira/browse/LUCENE-3312?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13432041#comment-13432041 ]
Michael McCandless commented on LUCENE-3312: -------------------------------------------- Branch + current patch looks great! I just found some minor things: IndexDocument, Document methods (eg the new clear()), GeneralField need javadocs. Import statements should be under the copyright header (eg StoredDocument.java, StorableField.java, GeneralField.java, StoredFieldsWriter.java)? Silly IDEs... Emacs does this correctly ;) Document's add(IndexableField) and add(StorableField) seem dangerous because they secretly cast to oal.document.Field? Ie, I cannot use Document to hold my private Storable/IndexableField implementations. I think we should remove them, leaving only add(Field)? I think StoredDocument should be in oal.index not oal.document? Ie, because it's something you've retrieved from the IndexReader. Also, it will cause confusion with oal.document.Document which is the obvious class you should use to hold all your indexed/stored fields. Why does StoredDocument still have removeField/s? Shouldn't it be read-only? (I feel like a broken record....). > Break out StorableField from IndexableField > ------------------------------------------- > > Key: LUCENE-3312 > URL: https://issues.apache.org/jira/browse/LUCENE-3312 > Project: Lucene - Core > Issue Type: Improvement > Components: core/index > Reporter: Michael McCandless > Assignee: Nikola Tankovic > Labels: gsoc2012, lucene-gsoc-12 > Fix For: Field Type branch > > Attachments: lucene-3312-patch-01.patch, lucene-3312-patch-02.patch, > lucene-3312-patch-03.patch, lucene-3312-patch-04.patch, > lucene-3312-patch-05.patch, lucene-3312-patch-06.patch, > lucene-3312-patch-07.patch, lucene-3312-patch-08.patch, > lucene-3312-patch-09.patch, lucene-3312-patch-10.patch > > > In the field type branch we have strongly decoupled > Document/Field/FieldType impl from the indexer, by having only a > narrow API (IndexableField) passed to IndexWriter. This frees apps up > use their own "documents" instead of the "user-space" impls we provide > in oal.document. > Similarly, with LUCENE-3309, we've done the same thing on the > doc/field retrieval side (from IndexReader), with the > StoredFieldsVisitor. > But, maybe we should break out StorableField from IndexableField, > such that when you index a doc you provide two Iterables -- one for the > IndexableFields and one for the StorableFields. Either can be null. > One downside is possible perf hit for fields that are both indexed & > stored (ie, we visit them twice, lookup their name in a hash twice, > etc.). But the upside is a cleaner separation of concerns in API.... -- 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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org