dajac commented on code in PR #14418:
URL: https://github.com/apache/kafka/pull/14418#discussion_r1333051894


##########
server-common/src/main/java/org/apache/kafka/server/common/TopicIdPartition.java:
##########
@@ -14,43 +14,62 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+package org.apache.kafka.server.common;
 
-package org.apache.kafka.controller;
-
-import java.util.Objects;
 import org.apache.kafka.common.Uuid;
 
-final class TopicIdPartition {
+/**
+ * Represents a partition using its unique topic Id and partition number.
+ */
+public final class TopicIdPartition {
     private final Uuid topicId;
-    private final int partitionId;
+    private final Integer partitionId;

Review Comment:
   Why changing to an Integer? An int seems more appropriate.



##########
server-common/src/main/java/org/apache/kafka/server/common/TopicIdPartition.java:
##########
@@ -14,43 +14,62 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+package org.apache.kafka.server.common;
 
-package org.apache.kafka.controller;
-
-import java.util.Objects;
 import org.apache.kafka.common.Uuid;
 
-final class TopicIdPartition {
+/**
+ * Represents a partition using its unique topic Id and partition number.
+ */
+public final class TopicIdPartition {
     private final Uuid topicId;
-    private final int partitionId;
+    private final Integer partitionId;
 
-    TopicIdPartition(Uuid topicId, int partitionId) {
+    public TopicIdPartition(Uuid topicId, int partitionId) {
         this.topicId = topicId;
         this.partitionId = partitionId;
     }
 
+    /**
+     * @return Universally unique Id representing this topic partition.
+     */
     public Uuid topicId() {
         return topicId;
     }
 
+    /**
+     * @return The partition Id.
+     */
     public int partitionId() {
         return partitionId;
     }
 
     @Override
     public boolean equals(Object o) {
-        if (!(o instanceof TopicIdPartition)) return false;
-        TopicIdPartition other = (TopicIdPartition) o;
-        return other.topicId.equals(topicId) && other.partitionId == 
partitionId;
+        if (this == o) {

Review Comment:
   nit: Should we keep the previous implementation for equals, hashCode and 
toString? We just want to move the class so we should not change it.



##########
server-common/src/main/java/org/apache/kafka/server/common/TopicIdPartition.java:
##########
@@ -14,43 +14,62 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+package org.apache.kafka.server.common;
 
-package org.apache.kafka.controller;
-
-import java.util.Objects;
 import org.apache.kafka.common.Uuid;
 
-final class TopicIdPartition {
+/**
+ * Represents a partition using its unique topic Id and partition number.
+ */
+public final class TopicIdPartition {
     private final Uuid topicId;
-    private final int partitionId;
+    private final Integer partitionId;
 
-    TopicIdPartition(Uuid topicId, int partitionId) {
+    public TopicIdPartition(Uuid topicId, int partitionId) {
         this.topicId = topicId;
         this.partitionId = partitionId;
     }
 
+    /**
+     * @return Universally unique Id representing this topic partition.
+     */
     public Uuid topicId() {
         return topicId;
     }
 
+    /**
+     * @return The partition Id.
+     */
     public int partitionId() {
         return partitionId;
     }
 
     @Override
     public boolean equals(Object o) {
-        if (!(o instanceof TopicIdPartition)) return false;
-        TopicIdPartition other = (TopicIdPartition) o;
-        return other.topicId.equals(topicId) && other.partitionId == 
partitionId;
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        TopicIdPartition that = (TopicIdPartition) o;
+        return topicId.equals(that.topicId) &&
+            partitionId.equals(that.partitionId);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(topicId, partitionId);
+        final int prime = 31;
+        int result = prime + topicId.hashCode();
+        result = prime * result + partitionId.hashCode();
+        return result;
     }
 
     @Override
     public String toString() {
-        return topicId + ":" + partitionId;
+        return "TopicIdPartition(" +
+            "topicId=" + topicId +
+            ", partition=" + partitionId +
+            ')';
     }
-}
+}

Review Comment:
   Let's add an empty line at the end of the file as it was.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to