You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2021/12/16 20:13:50 UTC
[GitHub] [cassandra] maedhroz commented on a change in pull request #1363: Avoid unecessary array allocations and initializations when performin…
maedhroz commented on a change in pull request #1363:
URL: https://github.com/apache/cassandra/pull/1363#discussion_r770741930
##########
File path: src/java/org/apache/cassandra/cql3/statements/RequestValidations.java
##########
@@ -26,10 +26,24 @@
import org.apache.cassandra.exceptions.InvalidRequestException;
import org.apache.cassandra.utils.ByteBufferUtil;
-import static org.apache.commons.lang3.ArrayUtils.EMPTY_OBJECT_ARRAY;
-
/**
* Utility methods use to perform request validation.
+ *
+ * <p>This class use overloaded methods to allow to specify different numbers of message arguments. While
+ * this introduces some clutter in the API, it avoids array allocation, initialization, and garbage collection
+ * overhead that is incurred by varargs calls. </p>
+ *
+ * <b>Warning about performance</b>
+ *
+ * <p>The goal of this class is to improve readability of code, but in some circumstances this may come at a
+ * significant performance cost. Remember that argument values for message construction must all be computed eagerly,
+ * and autoboxing may happen as well, even when the check succeeds. If the message arguments are expensive to create
+ * you should use the customary form:
+ * <pre>
+ * if (value < 0.0) {
Review comment:
nit:
```suggestion
* if (value < 0.0)
```
##########
File path: src/java/org/apache/cassandra/cql3/statements/RequestValidations.java
##########
@@ -180,14 +326,13 @@ public static void checkBindValueSet(ByteBuffer b, String messageTemplate, Objec
*
* @param object the object to test
* @param messageTemplate the template used to build the error message
- * @param messageArgs the message arguments
+ * @param messageArg the message argument
* @return the object
* @throws InvalidRequestException if the specified object is not <code>null</code>.
*/
- public static <T> T checkNull(T object, String messageTemplate, Object... messageArgs)
- throws InvalidRequestException
+ public static <T> T checkNull(T object, String messageTemplate, Object messageArg) throws InvalidRequestException
{
- checkTrue(object == null, messageTemplate, messageArgs);
+ checkTrue(object == null, messageTemplate, messageArg);
return object;
Review comment:
nit: I think this is always `null` if we get here. Not really a problem, but might be more obvious if we just return `null`.
##########
File path: src/java/org/apache/cassandra/cql3/statements/RequestValidations.java
##########
@@ -125,38 +255,55 @@ public static void checkFalse(boolean expression, String message) throws Invalid
checkTrue(!expression, message);
}
+ /**
+ * Checks that the specified object is NOT <code>null</code>.
+ * If it is an <code>InvalidRequestException</code> will be throws.
Review comment:
```suggestion
* If it is an <code>InvalidRequestException</code> will be thrown.
```
##########
File path: src/java/org/apache/cassandra/cql3/statements/RequestValidations.java
##########
@@ -52,16 +67,95 @@ public static void checkTrue(boolean expression, String message) throws InvalidR
*
* @param expression the expression to test
* @param messageTemplate the template used to build the error message
- * @param messageArgs the message arguments
+ * @param messageArg the message argument
+ * @throws InvalidRequestException if the specified expression is <code>false</code>.
+ */
+ public static void checkTrue(boolean expression,
+ String messageTemplate,
+ Object messageArg) throws InvalidRequestException
+ {
+ if (!expression)
+ throw invalidRequest(messageTemplate, messageArg);
+ }
+
+ /**
+ * Checks that the specified expression is <code>true</code>. If not an <code>InvalidRequestException</code> will
Review comment:
nit: It might be nice to use `@link` rather than `code` so things are navigable?
##########
File path: src/java/org/apache/cassandra/cql3/statements/RequestValidations.java
##########
@@ -52,16 +67,95 @@ public static void checkTrue(boolean expression, String message) throws InvalidR
*
* @param expression the expression to test
* @param messageTemplate the template used to build the error message
- * @param messageArgs the message arguments
+ * @param messageArg the message argument
+ * @throws InvalidRequestException if the specified expression is <code>false</code>.
+ */
+ public static void checkTrue(boolean expression,
+ String messageTemplate,
+ Object messageArg) throws InvalidRequestException
+ {
+ if (!expression)
+ throw invalidRequest(messageTemplate, messageArg);
+ }
+
+ /**
+ * Checks that the specified expression is <code>true</code>. If not an <code>InvalidRequestException</code> will
+ * be thrown.
+ *
+ * @param expression the expression to test
+ * @param messageTemplate the template used to build the error message
+ * @param firstArg the first message argument
+ * @param secondArg the second message argument
+ * @throws InvalidRequestException if the specified expression is <code>false</code>.
+ */
+ public static void checkTrue(boolean expression,
+ String messageTemplate,
+ Object firstArg,
+ Object secondArg) throws InvalidRequestException
+ {
+ if (!expression)
+ throw invalidRequest(messageTemplate, firstArg, secondArg);
+ }
+
+ /**
+ * Checks that the specified expression is <code>true</code>. If not an <code>InvalidRequestException</code> will
+ * be thrown.
+ *
+ * @param expression the expression to test
+ * @param messageTemplate the template used to build the error message
+ * @param firstArg the first message argument
+ * @param secondArg the second message argument
+ * @param thirdArg the third message argument
* @throws InvalidRequestException if the specified expression is <code>false</code>.
*/
public static void checkTrue(boolean expression,
String messageTemplate,
- Object... messageArgs)
- throws InvalidRequestException
+ Object firstArg,
+ Object secondArg,
+ Object thirdArg) throws InvalidRequestException
{
if (!expression)
- throw invalidRequest(messageTemplate, messageArgs);
+ throw invalidRequest(messageTemplate, firstArg, secondArg, thirdArg);
+ }
+
+ /**
+ * Checks that the specified collections is NOT <code>empty</code>.
+ * If it is an <code>InvalidRequestException</code> will be throws.
Review comment:
```suggestion
* If it is an <code>InvalidRequestException</code> will be thrown.
```
##########
File path: src/java/org/apache/cassandra/cql3/statements/RequestValidations.java
##########
@@ -52,16 +67,95 @@ public static void checkTrue(boolean expression, String message) throws InvalidR
*
* @param expression the expression to test
* @param messageTemplate the template used to build the error message
- * @param messageArgs the message arguments
+ * @param messageArg the message argument
+ * @throws InvalidRequestException if the specified expression is <code>false</code>.
+ */
+ public static void checkTrue(boolean expression,
+ String messageTemplate,
+ Object messageArg) throws InvalidRequestException
+ {
+ if (!expression)
+ throw invalidRequest(messageTemplate, messageArg);
+ }
+
+ /**
+ * Checks that the specified expression is <code>true</code>. If not an <code>InvalidRequestException</code> will
+ * be thrown.
+ *
+ * @param expression the expression to test
+ * @param messageTemplate the template used to build the error message
+ * @param firstArg the first message argument
+ * @param secondArg the second message argument
+ * @throws InvalidRequestException if the specified expression is <code>false</code>.
+ */
+ public static void checkTrue(boolean expression,
+ String messageTemplate,
+ Object firstArg,
+ Object secondArg) throws InvalidRequestException
+ {
+ if (!expression)
+ throw invalidRequest(messageTemplate, firstArg, secondArg);
+ }
+
+ /**
+ * Checks that the specified expression is <code>true</code>. If not an <code>InvalidRequestException</code> will
+ * be thrown.
+ *
+ * @param expression the expression to test
+ * @param messageTemplate the template used to build the error message
+ * @param firstArg the first message argument
+ * @param secondArg the second message argument
+ * @param thirdArg the third message argument
* @throws InvalidRequestException if the specified expression is <code>false</code>.
*/
public static void checkTrue(boolean expression,
String messageTemplate,
- Object... messageArgs)
- throws InvalidRequestException
+ Object firstArg,
+ Object secondArg,
+ Object thirdArg) throws InvalidRequestException
{
if (!expression)
- throw invalidRequest(messageTemplate, messageArgs);
+ throw invalidRequest(messageTemplate, firstArg, secondArg, thirdArg);
+ }
+
+ /**
+ * Checks that the specified collections is NOT <code>empty</code>.
+ * If it is an <code>InvalidRequestException</code> will be throws.
+ *
+ * @param collection the collection to test
+ * @param messageTemplate the template used to build the error message
+ * @param messageArg the message argument
+ * @return the collection
+ * @throws InvalidRequestException if the specified collection is <code>empty</code>.
+ */
+ public static <T extends Collection<E>, E> T checkNotEmpty(T collection,
+ String messageTemplate,
+ Object messageArg)
+ throws InvalidRequestException
+ {
+ checkTrue(!collection.isEmpty(), messageTemplate, messageArg);
+ return collection;
+ }
+
+ /**
+ * Checks that the specified collections is NOT <code>empty</code>.
+ * If it is an <code>InvalidRequestException</code> will be throws.
Review comment:
```suggestion
* If it is an <code>InvalidRequestException</code> will be thrown.
```
##########
File path: src/java/org/apache/cassandra/cql3/statements/RequestValidations.java
##########
@@ -52,16 +67,95 @@ public static void checkTrue(boolean expression, String message) throws InvalidR
*
* @param expression the expression to test
* @param messageTemplate the template used to build the error message
- * @param messageArgs the message arguments
+ * @param messageArg the message argument
+ * @throws InvalidRequestException if the specified expression is <code>false</code>.
Review comment:
nit: Might be easier to read quickly with a newline between the last `@params` and the first `@throws`
##########
File path: src/java/org/apache/cassandra/cql3/statements/RequestValidations.java
##########
@@ -52,16 +67,95 @@ public static void checkTrue(boolean expression, String message) throws InvalidR
*
* @param expression the expression to test
* @param messageTemplate the template used to build the error message
- * @param messageArgs the message arguments
+ * @param messageArg the message argument
+ * @throws InvalidRequestException if the specified expression is <code>false</code>.
+ */
+ public static void checkTrue(boolean expression,
+ String messageTemplate,
+ Object messageArg) throws InvalidRequestException
+ {
+ if (!expression)
+ throw invalidRequest(messageTemplate, messageArg);
+ }
+
+ /**
+ * Checks that the specified expression is <code>true</code>. If not an <code>InvalidRequestException</code> will
+ * be thrown.
+ *
+ * @param expression the expression to test
+ * @param messageTemplate the template used to build the error message
+ * @param firstArg the first message argument
+ * @param secondArg the second message argument
+ * @throws InvalidRequestException if the specified expression is <code>false</code>.
+ */
+ public static void checkTrue(boolean expression,
+ String messageTemplate,
+ Object firstArg,
+ Object secondArg) throws InvalidRequestException
+ {
+ if (!expression)
+ throw invalidRequest(messageTemplate, firstArg, secondArg);
+ }
+
+ /**
+ * Checks that the specified expression is <code>true</code>. If not an <code>InvalidRequestException</code> will
+ * be thrown.
+ *
+ * @param expression the expression to test
+ * @param messageTemplate the template used to build the error message
+ * @param firstArg the first message argument
+ * @param secondArg the second message argument
+ * @param thirdArg the third message argument
* @throws InvalidRequestException if the specified expression is <code>false</code>.
*/
public static void checkTrue(boolean expression,
String messageTemplate,
- Object... messageArgs)
- throws InvalidRequestException
+ Object firstArg,
+ Object secondArg,
+ Object thirdArg) throws InvalidRequestException
Review comment:
style: If you're looking for something less verbose, `arg1`, `arg2`, `arg3` is probably an acceptable naming scheme too.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org