[GitHub] [zookeeper] belugabehr commented on a change in pull request #1193: ZOOKEEPER-3663: Clean Up ZNodeName Class

[GitBox <gi...@apache.org>]

belugabehr commented on a change in pull request #1193: ZOOKEEPER-3663: Clean Up ZNodeName Class
URL: https://github.com/apache/zookeeper/pull/1193#discussion_r361213861
 
 

 ##########
 File path: zookeeper-recipes/zookeeper-recipes-lock/src/main/java/org/apache/zookeeper/recipes/lock/ZNodeName.java
 ##########
 @@ -18,45 +18,62 @@
 
 package org.apache.zookeeper.recipes.lock;
 
+import java.util.Objects;
+import java.util.Optional;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Represents an ephemeral znode name which has an ordered sequence number
- * and can be sorted in order.
+ * Represents an immutable ephemeral znode name which has an ordered sequence
+ * number and can be sorted in order. The expected name format of the znode is
+ * as follows:
  *
+ * <pre>
+ * &lt;name&gt;-&lt;sequence&gt;
+ *
+ * For example: lock-00001
+ * </pre>
  */
 class ZNodeName implements Comparable<ZNodeName> {
 
-    private final String name;
-    private String prefix;
-    private int sequence = -1;
     private static final Logger LOG = LoggerFactory.getLogger(ZNodeName.class);
 
-    public ZNodeName(String name) {
-        if (name == null) {
-            throw new NullPointerException("id cannot be null");
-        }
-        this.name = name;
-        this.prefix = name;
-        int idx = name.lastIndexOf('-');
-        if (idx >= 0) {
+    private final String name;
+    private final String prefix;
+    private final Optional<Integer> sequence;
 
 Review comment:
   @eolivelli I always prefer using Optional.  It's always very clear as to its usage and the caller must deal with it.  Better than forcing the user to correctly check for a special negative value that indicates 'no sequence available'.  This is not performance-critical code, so readability and usability should be prioritized.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services