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

Robert Muir commented on LUCENE-8326:
-------------------------------------

First looking at the API change, it would be good to understand the goals. 

This change wraps 8 or 9 existing setters such as {{setMinTermLen}} with a 
"configuration class". There is also another class related to boosts. But 
everything is still just as mutable as before, so from my perspective it only 
adds additional indirection/abstraction which is undesired.

If we want to make MLT immutable or something like that, we should first figure 
out if that's worth it. From my perspective, I'm not sold on this for 
MoreLikeThis itself, since its lightweight and stateless, and since I can't see 
a way for MoreLikeThisQuery to cache efficiently.

On the other hand MoreLikeThisQuery is kind of a mess, but that isn't addressed 
with the refactoring. Really all queries should be immutable for caching 
purposes, and should all have correct equals/hashcode: but seems like its a 
lost cause with MoreLikeThisQuery since it does strange stuff in rewrite: its 
not really a per-segment thing. Because of how the query works, its not obvious 
to me if/how we can improve it with immutability...

Also currently MoreLikeThisQuery doesn't accept MoreLikeThis as a parameter or 
anything, and only uses it internally. So as it stands (also with this patch) 
it still has a "duplicate" API of all the parameters, which isn't great.

So I think if we want to change the API for this stuff, we should figure out 
what the goals are? If its just to say, consolidate api between MoreLikeThis 
and MoreLikeThisQuery, I can buy into that (although I have never used the 
latter myself, only the former). However the other queries use builders for 
such purposes so that's probably something to consider.

For the solr changes, my only comment would be that instead of running actual 
queries, isn't it good enough to just test that XYZ configuration produces a 
correct MLT object? Otherwise the test seems fragile from my perspective.

> More Like This Params Refactor
> ------------------------------
>
>                 Key: LUCENE-8326
>                 URL: https://issues.apache.org/jira/browse/LUCENE-8326
>             Project: Lucene - Core
>          Issue Type: Improvement
>            Reporter: Alessandro Benedetti
>            Priority: Major
>         Attachments: LUCENE-8326.patch, LUCENE-8326.patch
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> More Like This ca be refactored to improve the code readability, test 
> coverage and maintenance.
> Scope of this Jira issue is to start the More Like This refactor from the 
> More Like This Params.
> This Jira will not improve the current More Like This but just keep the same 
> functionality with a refactored code.
> Other Jira issues will follow improving the overall code readability, test 
> coverage and maintenance.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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

Reply via email to