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

Reply via email to