[GitHub] [incubator-datasketches-java] davecromberge commented on a change in pull request #329: Rename "simpleIntLog2" to "simpleLog2OfLong" plus javadoc changes.

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

davecromberge commented on a change in pull request #329:
URL: https://github.com/apache/incubator-datasketches-java/pull/329#discussion_r465558585



##########
File path: src/main/java/org/apache/datasketches/Util.java
##########
@@ -452,14 +453,36 @@ public static int ceilingPowerOf2(final int n) {
    * integer.</li>
    * </ul>
    *
-   * @param n The given argument.
-   * @return the floor power of 2.
+   * @param n The given int argument.
+   * @return the floor power of 2 as an int.
    */
   public static int floorPowerOf2(final int n) {
     if (n <= 1) { return 1; }
     return Integer.highestOneBit(n);
   }
 
+  /**
+   * Computes the floor power of 2 given <i>n</i> is in therange [1, 2^63-1].
+   * This is the largest positive power of 2 that equal to or less than the given n and equal
+   * to a mathematical integer.

Review comment:
       There is a slight typo in the javadoc for this and the previous method:
   ```This is the largest positive power of 2 that _is_ equal```

##########
File path: src/main/java/org/apache/datasketches/Util.java
##########
@@ -452,14 +453,36 @@ public static int ceilingPowerOf2(final int n) {
    * integer.</li>
    * </ul>
    *
-   * @param n The given argument.
-   * @return the floor power of 2.
+   * @param n The given int argument.
+   * @return the floor power of 2 as an int.
    */
   public static int floorPowerOf2(final int n) {
     if (n <= 1) { return 1; }
     return Integer.highestOneBit(n);
   }
 
+  /**
+   * Computes the floor power of 2 given <i>n</i> is in therange [1, 2^63-1].
+   * This is the largest positive power of 2 that equal to or less than the given n and equal
+   * to a mathematical integer.
+   *
+   * <p>For:
+   * <ul>
+   * <li>n &le; 1: returns 1</li>
+   * <li>2^62 &le; n &le; 2^63 -1 : returns 2^62</li>
+   * <li>n == a power of 2 : returns n</li>
+   * <li>otherwise returns the largest power of 2 less than n and equal to a mathematical
+   * integer.</li>
+   * </ul>
+   *
+   * @param n The given long argument.
+   * @return the floor power of 2 as a long
+   */
+  public static long floorPowerOf2(final long n) {
+    if (n <= 1) { return 1; }

Review comment:
       Could returning a sentinel value of 1 lead to unexpected errors for n less than or equal to one?  I am curious why the zeroth power is not returned instead for this case - is 1 used for identity purposes?




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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org