renatoh commented on code in PR #15704:
URL: https://github.com/apache/lucene/pull/15704#discussion_r2855708355
##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/Word2VecSynonymProvider.java:
##########
@@ -66,14 +67,14 @@ public Word2VecSynonymProvider(Word2VecModel model) throws
IOException {
this.hnswGraph = builder.build(word2VecModel.size());
}
- public List<TermAndBoost> getSynonyms(
+ public Collection<TermAndBoost> getSynonyms(
Review Comment:
@uschindler Thanks for your feedback.
As far as I understand how synonyms works in Lucenen, the order should never
matter since all of the synonyms will be at the same position in the index.
Given that, Collection would be the right choice. But feel free to prove me
wrong;)
##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/Word2VecSynonymFilter.java:
##########
@@ -91,9 +92,9 @@ public boolean incrementToken() throws IOException {
BytesRefBuilder bytesRefBuilder = new BytesRefBuilder();
bytesRefBuilder.copyChars(termAtt.buffer(), 0, termAtt.length());
BytesRef term = bytesRefBuilder.get();
- List<TermAndBoost> synonyms =
+ Collection<TermAndBoost> synonyms =
this.synonymProvider.getSynonyms(term, maxSynonymsPerTerm,
minAcceptedSimilarity);
- if (synonyms.size() > 0) {
+ if (!synonyms.isEmpty()) {
Review Comment:
so the people in the Lucene world prefer to implement their own logic
instead of calling a Java core method which has the logic in it?;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
