jerqi commented on code in PR #6912:
URL: https://github.com/apache/gravitino/pull/6912#discussion_r2041359903


##########
api/src/main/java/org/apache/gravitino/model/ModelChange.java:
##########
@@ -97,4 +119,138 @@ public String toString() {
       return "RenameModel " + newName;
     }
   }
+
+  /** A ModelChange to set a property and value of a model. */
+  final class SetProperty implements ModelChange {
+    private final String property;
+    private final String value;
+
+    /**
+     * Constructs a new {@link SetProperty} instance with the specified 
property name and value.
+     *
+     * @param property The name of the property to be set.
+     * @param value The value to be set for the property.
+     */
+    public SetProperty(String property, String value) {
+      this.property = property;
+      this.value = value;
+    }
+
+    /**
+     * Retrieves the name of the property to be set.
+     *
+     * @return The name of the property to be set.
+     */
+    public String property() {
+      return property;
+    }
+
+    /**
+     * Retrieves the value to be set for the property.
+     *
+     * @return The value to be set for the property.
+     */
+    public String value() {
+      return value;
+    }
+
+    /**
+     * Compares this SetProperty instance with another object for equality. 
Two instances are
+     * considered equal if they target the same property and set the same 
value.
+     *
+     * @param obj The object to compare with this instance.
+     * @return {@code true} if the given object represents the same property 
set; {@code false}
+     *     otherwise.
+     */
+    @Override
+    public boolean equals(Object obj) {
+      if (obj == this) return true;
+      if (obj == null || getClass() != obj.getClass()) return false;

Review Comment:
   You can use `obj instanceOf SetProperty ` directly. Null will return false. 
It's more clear. This is the suggestion of <<Effective Java>.>



##########
api/src/main/java/org/apache/gravitino/model/ModelChange.java:
##########
@@ -97,4 +119,138 @@ public String toString() {
       return "RenameModel " + newName;
     }
   }
+
+  /** A ModelChange to set a property and value of a model. */
+  final class SetProperty implements ModelChange {
+    private final String property;
+    private final String value;
+
+    /**
+     * Constructs a new {@link SetProperty} instance with the specified 
property name and value.
+     *
+     * @param property The name of the property to be set.
+     * @param value The value to be set for the property.
+     */
+    public SetProperty(String property, String value) {
+      this.property = property;
+      this.value = value;
+    }
+
+    /**
+     * Retrieves the name of the property to be set.
+     *
+     * @return The name of the property to be set.
+     */
+    public String property() {
+      return property;
+    }
+
+    /**
+     * Retrieves the value to be set for the property.
+     *
+     * @return The value to be set for the property.
+     */
+    public String value() {
+      return value;
+    }
+
+    /**
+     * Compares this SetProperty instance with another object for equality. 
Two instances are
+     * considered equal if they target the same property and set the same 
value.
+     *
+     * @param obj The object to compare with this instance.
+     * @return {@code true} if the given object represents the same property 
set; {@code false}
+     *     otherwise.
+     */
+    @Override
+    public boolean equals(Object obj) {
+      if (obj == this) return true;
+      if (obj == null || getClass() != obj.getClass()) return false;
+      SetProperty other = (SetProperty) obj;
+      return property.equals(other.property) && value.equals(other.value);
+    }
+
+    /**
+     * Generates a hash code for this SetProperty instance. The hash code is 
based on the property
+     * name and value to be set.
+     *
+     * @return A hash code value for this property set operation.
+     */
+    @Override
+    public int hashCode() {
+      return Objects.hash(property, value);
+    }
+
+    /**
+     * Provides a string representation of the SetProperty instance. This 
string format includes the
+     * class name followed by the property name and value to be set.
+     *
+     * @return A string summary of the property set operation.
+     */
+    @Override
+    public String toString() {
+      return "SETPROPERTY " + property + " " + value;
+    }
+  }
+
+  /** A ModelChange to remove a property from model. */
+  final class RemoveProperty implements ModelChange {
+    private final String property;
+
+    /**
+     * Constructs a new {@link RemoveProperty} instance with the specified 
property name.
+     *
+     * @param property The name of the property to be removed from the model.
+     */
+    public RemoveProperty(String property) {
+      this.property = property;
+    }
+
+    /**
+     * Retrieves the name of the property to be removed from the model.
+     *
+     * @return The name of the property for removal.
+     */
+    public String property() {
+      return property;
+    }
+
+    /**
+     * Compares this RemoveProperty instance with another object for equality. 
Two instances are
+     * considered equal if they target the same property for removal from the 
fileset.
+     *
+     * @param obj The object to compare with this instance.
+     * @return {@code true} if the given object represents the same property 
removal; {@code false}
+     *     otherwise.
+     */
+    @Override
+    public boolean equals(Object obj) {
+      if (obj == this) return true;
+      if (obj == null || getClass() != obj.getClass()) return false;

Review Comment:
   ditto.



##########
api/src/main/java/org/apache/gravitino/model/ModelChange.java:
##########
@@ -97,4 +119,138 @@ public String toString() {
       return "RenameModel " + newName;
     }
   }
+
+  /** A ModelChange to set a property and value of a model. */
+  final class SetProperty implements ModelChange {
+    private final String property;
+    private final String value;
+
+    /**
+     * Constructs a new {@link SetProperty} instance with the specified 
property name and value.
+     *
+     * @param property The name of the property to be set.
+     * @param value The value to be set for the property.
+     */
+    public SetProperty(String property, String value) {
+      this.property = property;
+      this.value = value;
+    }
+
+    /**
+     * Retrieves the name of the property to be set.
+     *
+     * @return The name of the property to be set.
+     */
+    public String property() {
+      return property;
+    }
+
+    /**
+     * Retrieves the value to be set for the property.
+     *
+     * @return The value to be set for the property.
+     */
+    public String value() {
+      return value;
+    }
+
+    /**
+     * Compares this SetProperty instance with another object for equality. 
Two instances are
+     * considered equal if they target the same property and set the same 
value.
+     *
+     * @param obj The object to compare with this instance.
+     * @return {@code true} if the given object represents the same property 
set; {@code false}
+     *     otherwise.
+     */
+    @Override
+    public boolean equals(Object obj) {
+      if (obj == this) return true;
+      if (obj == null || getClass() != obj.getClass()) return false;
+      SetProperty other = (SetProperty) obj;
+      return property.equals(other.property) && value.equals(other.value);

Review Comment:
   Could the property and value be null? Will it throw NullPointerException? 



-- 
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