[GitHub] [ignite-3] korlov42 commented on a diff in pull request #965: IGNITE-17439 Small enhancements for BinaryTuple

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

korlov42 commented on code in PR #965:
URL: https://github.com/apache/ignite-3/pull/965#discussion_r937552234


##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTupleSchema.java:
##########
@@ -289,10 +289,23 @@ public static int nullMapSize(int numElements) {
     }
 
     /**
-     * Returns the null map size in bytes if there are nullable elements, zero otherwise.
+     * Get offset of the byte that contains null-bit of a given tuple element.

Review Comment:
   I believe we should use 'Returns' verb here and below. You can _get_ the offset by calling this method, whereas the method _returns_ this value to you



##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTupleSchema.java:
##########
@@ -289,10 +289,23 @@ public static int nullMapSize(int numElements) {
     }
 
     /**
-     * Returns the null map size in bytes if there are nullable elements, zero otherwise.
+     * Get offset of the byte that contains null-bit of a given tuple element.
+     *
+     * @param index Tuple element index.
+     * @return Offset of the required byte relative to the tuple start.
+     */
+    public static int getNullOffset(int index) {
+        return HEADER_SIZE + index / 8;
+    }
+
+    /**
+     * Get a null-bit mask corresponding to a given tuple element.
+     *
+     * @param index Tuple element index.
+     * @return Mask to extract the required null-bit.
      */
-    public int nullMapSize() {
-        return hasNullableElements() ? nullMapSize(elementCount()) : 0;
+    public static byte getNullMask(int index) {

Review Comment:
   as far as I can see, vast majority of the methods within this class omit the `get` prefix. Lets follow this rule to make code more consistent



##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTupleSchema.java:
##########
@@ -289,10 +289,23 @@ public static int nullMapSize(int numElements) {
     }
 
     /**
-     * Returns the null map size in bytes if there are nullable elements, zero otherwise.
+     * Get offset of the byte that contains null-bit of a given tuple element.
+     *
+     * @param index Tuple element index.
+     * @return Offset of the required byte relative to the tuple start.
+     */
+    public static int getNullOffset(int index) {

Review Comment:
   lets rename this to `nullByteOffset`, because that name is better reflects the essence 



-- 
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: notifications-unsubscribe@ignite.apache.org

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