jiexray commented on code in PR #21410:
URL: https://github.com/apache/flink/pull/21410#discussion_r1094695776


##########
flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/ChangelogMapState.java:
##########
@@ -61,17 +62,7 @@
 
     private Map.Entry<UK, UV> loggingMapEntry(
             Map.Entry<UK, UV> entry, KvStateChangeLogger<Map<UK, UV>, N> 
changeLogger, N ns) {
-        return new Map.Entry<UK, UV>() {
-            @Override
-            public UK getKey() {
-                return entry.getKey();
-            }
-
-            @Override
-            public UV getValue() {
-                return entry.getValue();
-            }
-
+        return new AbstractMap.SimpleEntry<UK, UV>(entry.getKey(), 
entry.getValue()) {

Review Comment:
   The `assertThat(iterator.next()).isEqualTo(el);` in `ChangelogMapStateTest` 
uses `Object#equals` to compare two `Map.Entry`. The unnamed class 
`Map.Entry<UK, UV>` here uses the default `Object#equals()`, and fails the 
equal assertion.
   
   Alternatively, I add a basic `equal()` implementation for this unnamed Entry:
   ```
   @Override
   public boolean equals(Object o) {
       if (!(o instanceof Map.Entry)) {
           return false;
       }
       Map.Entry<?, ?> e = (Map.Entry<?, ?>)o;
       return Objects.equals(entry.getKey(), e.getKey()) && 
Objects.equals(entry.getValue(), e.getValue());
   }
   ```
   and remove the extension of `AbstractMap.SimpleEntry`.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to