Initial refactoring for phrase suggester#16241
Initial refactoring for phrase suggester#16241cbuescher wants to merge 1 commit intoelastic:masterfrom
Conversation
6afb09c to
8440320
Compare
There was a problem hiding this comment.
Didn't we at some point want to move towards genuine getters and setters?
There was a problem hiding this comment.
Since almost all of the setters here are not get/set I wanted to make it consistent without too much changes. I'm happy to rename them all together in one go at a later stage though. Also if anybody else objects I can change this back of course, I just don't want to change all the non-get/set type ones at this point.
There was a problem hiding this comment.
Yeah - makes sense. Maybe track the final move to get/set in a separate issue?
There was a problem hiding this comment.
What was the motivation for wanting to make them genuine "get/set" methods?
|
Left a few comments. Feel free to ignore the ones about methods being empty. I didn't realize this wasn't going against master. |
025391f to
600a8f8
Compare
There was a problem hiding this comment.
should be "get token separator"?
|
@cbuescher LGTM ++ |
There was a problem hiding this comment.
maybe we can make this private, I assume this is only for readability?
There was a problem hiding this comment.
Good point, will change that as well. And yes, currently only for readability. I hope we keep the same name constants as Id in the stream as well as for registering the parsers. That's what worked for queries at least.
|
@cbuescher LGTM. This looks great and thanks for the test infra :) |
Adding initial serialization methods (readFrom, writeTo) to the PhraseSuggestionBuilder, also adding the base test framework for serialiazation testing, equals and hashCode. Moving SuggestionBuilder out of the global SuggestBuilder for better readability.
29ca712 to
86db250
Compare
|
Didn't merge this with master but with feature-suggest-refactoring branch in c00c0fa. |
Adding initial serialization methods (readFrom, writeTo) to the PhraseSuggestionBuilder, also adding the base test framework for serialiazation testing, equals and hashCode. Moving SuggestionBuilder out of the global SuggestBuilder for better readability.
Still missing: Handling of Smoothing Model (depends on #16130) and DirectCandidateGenerator (depends on #16185). Also Test needs to be extended to handle all parameters.