Implements the option to use Regex on the Search Dialog #1069
Conversation
…ions set rather than method arguments
| * Checks if searchArea matches the searched string found in searchSettings | ||
| */ | ||
| public static boolean isMatch(String searchArea, final SearchSettings searchSettings) { | ||
| return find(searchArea, searchSettings) == -1 ? false : true; |
There was a problem hiding this comment.
This code is very likely to be misinterpreted by a developer. Why not clean and simple
return find(searchArea, searchSettings) != -1;
There was a problem hiding this comment.
Thanks not sure what I was thinking. Just submitted the change
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(SearchSettings.class); | ||
|
|
||
| private String searchString; |
There was a problem hiding this comment.
For efficiency I would recommend to make the fields final, remove the setters and provide for each use case an own constructor. As final fields can't change the Java code optimizer can generate faster code as several if branches can be omitted.
There was a problem hiding this comment.
I just implemented this for the searchString, ignoreCase, and useRegex option. It should work with a single constructor as all 3 options should always have a value.
| * Checks if searchArea matches the searched string found in searchSettings | ||
| */ | ||
| public static boolean isMatch(StringRef searchArea, final SearchSettings searchSettings) { | ||
| return isMatch(searchArea.toString(), searchSettings); |
There was a problem hiding this comment.
All your isMatch and find implementations are static and have an SearchSettings searchSettings argument.
So effectively they are handled like a virtual method of SearchSettings. I assume you wanted make a split between the data model and the implementation. Was this the reason or are there other reasons for doing so?
Because moving the code into SearchSettings would in the end allow to use SearchSettings as interface and provide for each major search type an own optimized implementation.
There was a problem hiding this comment.
Your correct in that my goal was to separate the data model from the implementation with the idea to build a settings class to store all options which can be expanded in the future if needed. Originally, I was thinking about just passing the settings class into the existing isMatched functions and doing the comparison there as was originally the case. Although, it appeared that codeIndex, simpleIndex, and the searchNext function all implement the same algorithm with minor changes so I just moved that into the searchImpl class rather than updating it in all 3 places.
I just submitted another commit combining SearchImpl and SearchSettings as you suggested and agree that it is cleaner than my original implementation. I still need to implement a search interface as recommended so will be making another commit.
What did you have in mind with doing an optimized implementation for each search type? Are you recommending to create a search interface then implement it in the index classes rather than using a generic find function implemented in the searchSettings class which is what I currently have.
This will implement the option to use Regex when searching with the text search dialog.
Addresses issue #954