mocobeta commented on a change in pull request #643: URL: https://github.com/apache/lucene/pull/643#discussion_r799940540
########## File path: lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/BinaryDictionary.java ########## @@ -154,6 +153,98 @@ protected BinaryDictionary(ResourceScheme resourceScheme, String resourcePath) this.buffer = buffer; } + protected BinaryDictionary( + Supplier<InputStream> targetMapResource, + Supplier<InputStream> posResource, + Supplier<InputStream> dictResource) + throws IOException { + this.resourceScheme = null; + this.resourcePath = null; + + int[] targetMapOffsets = null, targetMap = null; Review comment: Unfortunately, we can't call/delegate to other `this()` from the old constructor. A kind of chicken-egg problem is there - `getClass()` in it - this is needed to delegate to the newly added constructor that takes input streams, but it's a constructor so `getClass()` can't be invoked before `this()`. Instead, we can retire the old constructor and call `this()` or new `super()` in the implementation classes without changing public APIs. Anyway, it's not great to have the if-else for resource switching and the current `super()` call to delegate class resources loading; I think it is okay to remove the old constructor in the abstract `BinaryDictionary` right now? https://github.com/apache/lucene/pull/643/commits/9c2c971fae0cba6762fbbb0a7c62d1115e50b33d -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org