Proposal: Introduce requirePowerOfTwo Utility Method

[ata-nas]

Proposal

We currently have an existing check if an int is a power of two. It uses a bitwise operation to do so:

        if ((ringBufferSize & (ringBufferSize - 1)) != 0) {
            throw new IllegalArgumentException("Ring buffer size must be a power of 2");
        }

In my opinion this check is complex enough to become a method in a new utility class.
The logic is generic and is not coupled to any project.
It is exportable and reusable globally.
We have it already in a couple of places.

To Discuss

• Do you agree that this could be added into the common module?
• If we agree to add, what should we call the new class, maybe MathUtils or create a class Preconditions?
• if we call the new class Preconditions, should we move also the requireNotBlank method from the string utils there for consistency?

@mattp-swirldslabs @AlfredoG87 @jsync-swirlds @georgi-l95 What do you guys think?

[jsync-swirlds]

Adding a Preconditions or Validations class makes sense, as does moving the requireNotBlank method.
I'm not sure the proposed power of two implementation is completely correct, however; it probably needs some edge guards (e.g. value >= 0, value < MAX_INT, etc...).

[jsync-swirlds]

The require method is intended to mirror the Objects.requireNonNull method. I agree in principal, however.
Perhaps the following combination is acceptable:

public static boolean isPowerOfTwo(int value) {
   return ((value & (value - 1)) != 0);
}

public static void requirePowerOfTwo(int value) {
   if(!isPowerOfTwo(value)) throw new IllegalArgumentException("Argument must be a power of 2.");
}

[ata-nas]

@jsync-swirlds was going to propose that we have both as well, I think we should have both in general. That being said, as we discussed above, I think the Preconditions class should be also created and the require methods of the non-blank strings and this new check should go there. As far as the boolean method, I think should be added at a MathUtilities or similar class.

[jsync-swirlds]

Both methods makes sense in a preconditions class too; one for "hard" preconditions and another for "soft" preconditions.

[ata-nas]

@jsync-swirlds In that spirit, I would also propose to do the same for the strings:

// inside StringUtilities.java
public static boolean isBlank(final String toCheck) {
   return Objects.isNull(toCheck) || toCheck.isBlank();
 // shorthand for the null check as well, saves an OR every time we need to check if a string is blank
}

// inside Preconditions.java
public static void requireNotBlank(final String toCheck) {
   if(!isBlank(toCheck)) throw new IllegalArgumentException("Argument must not be blank!");
// or implemented like that ... `if Objects.requireNonNull().isBlank() throw...` as it is implemented now if you think is more appropriate that way 
}

[ata-nas]

@mattp-swirldslabs @jsync-swirlds since we have 4 upvotes, I will close this discussion and the proposed changes shall be implemented as discussed. See #305.