[GitHub] [kafka] dajac commented on a diff in pull request #14418: MINOR: Move TopicIdPartition class to server-common

2023-09-28 Thread via GitHub


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


##
server-common/src/main/java/org/apache/kafka/server/common/TopicIdPartition.java:
##
@@ -54,3 +63,6 @@ public String toString() {
 return topicId + ":" + partitionId;
 }
 }
+
+
+

Review Comment:
   nit: Could we remove those two empty lines? We just need one at the end of 
the file.



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



[GitHub] [kafka] dajac commented on a diff in pull request #14418: MINOR: Move TopicIdPartition class to server-common

2023-09-22 Thread via GitHub


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


##
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:
   Let's keep it 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



[GitHub] [kafka] dajac commented on a diff in pull request #14418: MINOR: Move TopicIdPartition class to server-common

2023-09-22 Thread via GitHub


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


##
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:
   We should keep an int here. It is possible to implement equals and hashCode 
with it (as it was before).



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



[GitHub] [kafka] dajac commented on a diff in pull request #14418: MINOR: Move TopicIdPartition class to server-common

2023-09-21 Thread via GitHub


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