[ 
https://issues.apache.org/jira/browse/OPENNLP-1412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17648091#comment-17648091
 ] 

ASF GitHub Bot commented on OPENNLP-1412:
-----------------------------------------

rzo1 commented on code in PR #458:
URL: https://github.com/apache/opennlp/pull/458#discussion_r1049705186


##########
opennlp-tools/src/main/java/opennlp/tools/parser/ParserModel.java:
##########
@@ -367,4 +367,27 @@ else if (ParserType.TREEINSERT.equals(modelType)) {
       throw new InvalidFormatException("Missing the head rules!");
     }
   }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(artifactMap.get("manifest.properties"),
+            artifactMap.get(PARSER_TAGGER_MODEL_ENTRY_NAME));
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (obj == this) {
+      return true;
+    }
+
+    if (obj instanceof ParserModel) {
+      ParserModel model = (ParserModel) obj;
+      Map<String, Object> artifactMapToCheck = model.artifactMap;
+      AbstractModel abstractModel = (AbstractModel) 
artifactMapToCheck.get(BUILD_MODEL_ENTRY_NAME);
+
+      return 
artifactMap.get("manifest.properties").equals(artifactMapToCheck.get("manifest.properties"))
 &&

Review Comment:
   see above



##########
opennlp-tools/src/main/java/opennlp/tools/tokenize/TokenizerModel.java:
##########
@@ -162,4 +163,26 @@ public Dictionary getAbbreviations() {
   public boolean useAlphaNumericOptimization() {
     return getFactory() != null && getFactory().isUseAlphaNumericOptmization();
   }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(artifactMap.get("manifest.properties"), 
artifactMap.get(TOKENIZER_MODEL_ENTRY));
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (obj == this) {
+      return true;
+    }
+
+    if (obj instanceof TokenizerModel) {
+      TokenizerModel model = (TokenizerModel) obj;
+      Map<String, Object> artifactMapToCheck = model.artifactMap;
+      AbstractModel abstractModel = (AbstractModel) 
artifactMapToCheck.get(TOKENIZER_MODEL_ENTRY);
+
+      return 
artifactMap.get("manifest.properties").equals(artifactMapToCheck.get("manifest.properties"))
 &&

Review Comment:
   see above



##########
opennlp-tools/src/main/java/opennlp/tools/parser/ParserModel.java:
##########
@@ -367,4 +367,27 @@ else if (ParserType.TREEINSERT.equals(modelType)) {
       throw new InvalidFormatException("Missing the head rules!");
     }
   }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(artifactMap.get("manifest.properties"),

Review Comment:
   Can we use ` MANIFEST_ENTRY` from `BaseModel`  ?



##########
opennlp-tools/src/main/java/opennlp/tools/tokenize/TokenizerModel.java:
##########
@@ -162,4 +163,26 @@ public Dictionary getAbbreviations() {
   public boolean useAlphaNumericOptimization() {
     return getFactory() != null && getFactory().isUseAlphaNumericOptmization();
   }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(artifactMap.get("manifest.properties"), 
artifactMap.get(TOKENIZER_MODEL_ENTRY));

Review Comment:
   see above





> Provide equals and hashCode for ParserModel and TokenizerModel
> --------------------------------------------------------------
>
>                 Key: OPENNLP-1412
>                 URL: https://issues.apache.org/jira/browse/OPENNLP-1412
>             Project: OpenNLP
>          Issue Type: Improvement
>          Components: Parser, Tokenizer
>    Affects Versions: 2.1.0
>            Reporter: Martin Wiesner
>            Assignee: Martin Wiesner
>            Priority: Minor
>             Fix For: 2.1.1
>
>
> The tests {_}opennlp.tools.tokenize.{*}TokenizerModelTest{*}{_}, 
> {_}opennlp.tools.parser.chunking.{_}{*}_ParserTest_{*}, and 
> {_}opennlp.tools.parser.treeinsert.{_}{*}_ParserTest_{*} signal by TODOs that 
> no actual assertions are made to check whether (de-)serialized Model 
> instances are correctly read in again.
> In other words: Those (Tokenizer/Parser) models lacks a valid equals/hashCode 
> implementation by which one could verify a valid state (=equality) after 
> (de-)serialization has occurred.
> Aim:
>  * Provide an improved implementation of the related "Model" classes.
>  * Remove existing TODOs in _TokenizerModelTest_ and _ParserTest_
>  * Improve the three test classes with further test assertions.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to