Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/4876
  
    A quick post-mortem comment here:
    
    This adds a lot of `equals()` and `hashCode()` on classes where these are 
ill-defined.
    
    For example: `StreamWriterBase` defines `equals()` and `hashCode()` just on 
a subset of configuration fields (here the sync field) and ignore the 
associated stream, because equals and hash is ill-defined on the stream. To me, 
the correct conclusion is that `equals()` and `hashCode()` are ill-defined on 
`StreamWriterBase` and should not be there!
    
    Adding such methods just to make assertion statement in tests more compact 
wrongly pushes some specific test logic in to the main classes. The correct way 
is to adjust the assertions in the test, or, if there is a lot of repetitive 
checking, create a `Matcher` that matches "equality based on some fields" and 
replace `assertEquals(X, Y)` with `assertThat(matcher, X, Y)`.
    
    I think in this case, it is actually a reason for a follow-up patch that 
changes this.


---

Reply via email to