diqiu50 commented on code in PR #4378:
URL: https://github.com/apache/gravitino/pull/4378#discussion_r1704801646
##########
api/src/main/java/org/apache/gravitino/rel/expressions/distributions/Distribution.java:
##########
@@ -46,16 +46,29 @@ default Expression[] children() {
/**
* Indicates whether some other object is "equal to" this one.
*
- * @param distribution The reference distribution object with which to
compare.
+ * @param obj The reference object with which to compare.
* @return returns true if this object is the same as the obj argument;
false otherwise.
*/
- default boolean equals(Distribution distribution) {
- if (distribution == null) {
+ @Override
+ default boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if(obj == null || getClass() != obj.getClass()){
return false;
}
+ Distribution that = (Distribution) obj;
+ return number() == that.number()
+ && strategy().equals(that.strategy())
+ && Arrays.equals(expressions(), that.expressions());
+ }
- return strategy().equals(distribution.strategy())
- && number() == distribution.number()
- && Arrays.equals(expressions(), distribution.expressions());
+ @Override
+ default int hashCode() {
+ int result = strategy().hashCode();
+ result = 31 * result + number();
+ result = 31 * result + Arrays.hashCode(expressions());
+ return result;
+
}
Review Comment:
I think your change is good, but you need to maintain the original behavior
with your changes. This is because the comparison object may not be of one
class.
```
if (!Distributions.NONE.equals(distribution)) {
```
--
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]
