airborne12 commented on code in PR #43992: URL: https://github.com/apache/doris/pull/43992#discussion_r1942404513
########## be/src/olap/rowset/segment_v2/inverted_index_reader.cpp: ########## @@ -234,13 +237,9 @@ Status InvertedIndexReader::handle_searcher_cache( } } -Status InvertedIndexReader::create_index_searcher(lucene::store::Directory* dir, - IndexSearcherPtr* searcher, - InvertedIndexReaderType reader_type, - size_t& reader_size) { - auto index_searcher_builder = Review Comment: First, from a software design perspective, we should adhere to the Single Responsibility Principle (SRP). Therefore, the create_index_searcher function should only create a searcher, while the searcher builder acts as a factory method for creating a searcher. This separation clarifies each component’s responsibility. Second, from a unit testing perspective, we need to mock the searcher builder when testing. This approach allows us to avoid creating a real searcher to execute Lucene functionalities. -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org