[ https://issues.apache.org/jira/browse/LUCENE-3807?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13213477#comment-13213477 ]
Dawid Weiss commented on LUCENE-3807: ------------------------------------- Sorry for being late, work. I like the patch. Comments: + public Comparator<BytesRef> getComparator() { + return null; + } This shows up in a number of places. I have mixed feelings about certain types having a comparator and others not having it, but it's minor. BufferingTermFreqIteratorWrapper is a nuisance (buffers in memory). It would be nicer to have a sort on disk if something doesn't support sorted iteration order. I also wonder float -> long = 4 -> 8 bytes... would this count as an incompatible API change (because what used to work for a given amount of RAM won't work anymore -- BufferingTermFreqIteratorWrapper again)? + if (l1 < l2) { + aStop = l1; + } else { + aStop = l2; + } if I remember correctly Math.min/max are intrinsics, so you can afford to be explicit ;) Why not a specific covariant here? - public Float get(String key) { + public Object get(CharSequence key) { @@ -199,7 +199,7 @@ public class LookupBenchmarkTest extends LuceneTestCase { public Integer call() throws Exception { int v = 0; for (String term : input) { - v += lookup.lookup(term, onlyMorePopular, num).size(); + v += lookup.lookup(new CharsRef(term), onlyMorePopular, num).size(); This doesn't seem necessary (lookup accepts a CharSequence?). I like the rest, including the CharSequenceish evilness of bytesToCharSequence :) > Cleanup suggester API > --------------------- > > Key: LUCENE-3807 > URL: https://issues.apache.org/jira/browse/LUCENE-3807 > Project: Lucene - Java > Issue Type: Improvement > Components: modules/other > Affects Versions: 3.6, 4.0 > Reporter: Simon Willnauer > Fix For: 4.0 > > Attachments: LUCENE-3807.patch, LUCENE-3807.patch, LUCENE-3807.patch > > > Currently the suggester api and especially TermFreqIterator don't play that > nice with BytesRef and other paradigms we use in lucene, further the java > iterator pattern isn't that useful when it gets to work with TermsEnum, > BytesRef etc. We should try to clean up this api step by step moving over to > BytesRef including the Lookup class and its interface... -- 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