dcapwell commented on code in PR #2310:
URL: https://github.com/apache/cassandra/pull/2310#discussion_r1223274738


##########
src/java/org/apache/cassandra/db/marshal/ValueAccessor.java:
##########
@@ -311,6 +319,24 @@ default boolean getBoolean(V value, int offset)
     int toInt(V value);
     /** returns an int from offset {@param offset} */
     int getInt(V value, int offset);
+    default long getUnsignedVInt(V value, int offset)
+    {
+        return VIntCoding.getUnsignedVInt(value, this, offset);
+    }
+    default int getUnsignedVInt32(V value, int offset)
+    {
+        return VIntCoding.getUnsignedVInt32(value, this, offset);
+    }
+    default long getVInt(V value, int offset)
+    {
+        return VIntCoding.getVInt(value, this, offset);
+    }
+    default int getVInt32(V value, int offset)
+    {
+        return VIntCoding.getVInt32(value, this, offset);
+    }

Review Comment:
   > Of these new methods, only getUnsignedVInt32 seems used
   
   I think I only did it to just define things all at once so all allowed 
versions are supported... I know our style guide says not to add dead code, but 
felt it was best to have all get/put of each of the valid versions at once 
rather than require future authors to add... 
   
   I know the methods are tested in 
`org.apache.cassandra.utils.vint.VIntCodingTest` but I could always add tests 
for each method in `org.apache.cassandra.db.marshal.ValueAccessorTest`; I 
actually got happy to see these are property tests already =D



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to