essobedo commented on code in PR #9332:
URL: https://github.com/apache/camel/pull/9332#discussion_r1109451751


##########
core/camel-support/src/main/java/org/apache/camel/support/builder/ExpressionBuilder.java:
##########
@@ -1577,74 +1578,122 @@ public String toString() {
      * Returns an expression which returns the string concatenation value of 
the various
      * expressions
      *
+     * @param context the Camel context
      * @param expressions the expression to be concatenated dynamically
      * @return an expression which when evaluated will return the concatenated 
values
      */
-    public static Expression concatExpression(final Collection<Expression> 
expressions) {
-        return concatExpression(expressions, null);
+    public static Expression concatExpression(final CamelContext context, 
final Collection<Expression> expressions) {
+        return concatExpression(context, expressions, null);
     }
 
     /**
      * Returns an expression which returns the string concatenation value of 
the various
      * expressions
      *
+     * @param context the Camel context
      * @param expressions the expression to be concatenated dynamically
      * @param description the text description of the expression
      * @return an expression which when evaluated will return the concatenated 
values
      */
-    public static Expression concatExpression(final Collection<Expression> 
expressions, final String description) {
-        return new ExpressionAdapter() {
+    public static Expression concatExpression(final CamelContext context, 
final Collection<Expression> expressions, final String description) {
+
+        for (Expression expression : expressions) {
+            if(expression instanceof ConstantExpressionAdapter){
+                return concatExpressionOptimized(context, expressions, 
description);
+            }
+        }
 
-            private Collection<Object> col;
+        return concatExpression(expressions,description);
+    }
+
+    /**
+     * Returns an expression which returns the string concatenation value of 
the various
+     * expressions
+     *
+     * @param expressions the expression to be concatenated dynamically
+     * @param description the text description of the expression
+     * @return an expression which when evaluated will return the concatenated 
values
+     */
+    private static Expression concatExpression(final Collection<Expression> 
expressions, final String description) {
+        return new ExpressionAdapter() {
 
             @Override
             public Object evaluate(Exchange exchange) {
                 StringBuilder buffer = new StringBuilder();
-                if (col != null) {
-                    // optimize for constant expressions so we can do this a 
bit faster
-                    for (Object obj : col) {
-                        if (obj instanceof Expression) {
-                            Expression expression = (Expression) obj;
-                            String text = expression.evaluate(exchange, 
String.class);
-                            if (text != null) {
-                                buffer.append(text);
-                            }
-                        } else {
-                            buffer.append((String) obj);
-                        }
+                for (Expression expression : expressions) {
+                    String text = expression.evaluate(exchange, String.class);
+                    if (text != null) {
+                        buffer.append(text);
                     }
+                }
+                return buffer.toString();
+            }
+
+            @Override
+            public void init(CamelContext context) {
+                boolean constant = false;

Review Comment:
   Not needed



##########
core/camel-support/src/main/java/org/apache/camel/support/builder/ExpressionBuilder.java:
##########
@@ -1577,74 +1578,122 @@ public String toString() {
      * Returns an expression which returns the string concatenation value of 
the various
      * expressions
      *
+     * @param context the Camel context
      * @param expressions the expression to be concatenated dynamically
      * @return an expression which when evaluated will return the concatenated 
values
      */
-    public static Expression concatExpression(final Collection<Expression> 
expressions) {
-        return concatExpression(expressions, null);
+    public static Expression concatExpression(final CamelContext context, 
final Collection<Expression> expressions) {
+        return concatExpression(context, expressions, null);
     }
 
     /**
      * Returns an expression which returns the string concatenation value of 
the various
      * expressions
      *
+     * @param context the Camel context
      * @param expressions the expression to be concatenated dynamically
      * @param description the text description of the expression
      * @return an expression which when evaluated will return the concatenated 
values
      */
-    public static Expression concatExpression(final Collection<Expression> 
expressions, final String description) {
-        return new ExpressionAdapter() {
+    public static Expression concatExpression(final CamelContext context, 
final Collection<Expression> expressions, final String description) {
+
+        for (Expression expression : expressions) {
+            if(expression instanceof ConstantExpressionAdapter){
+                return concatExpressionOptimized(context, expressions, 
description);
+            }
+        }
 
-            private Collection<Object> col;
+        return concatExpression(expressions,description);
+    }
+
+    /**
+     * Returns an expression which returns the string concatenation value of 
the various
+     * expressions
+     *
+     * @param expressions the expression to be concatenated dynamically
+     * @param description the text description of the expression
+     * @return an expression which when evaluated will return the concatenated 
values
+     */
+    private static Expression concatExpression(final Collection<Expression> 
expressions, final String description) {
+        return new ExpressionAdapter() {
 
             @Override
             public Object evaluate(Exchange exchange) {
                 StringBuilder buffer = new StringBuilder();
-                if (col != null) {
-                    // optimize for constant expressions so we can do this a 
bit faster
-                    for (Object obj : col) {
-                        if (obj instanceof Expression) {
-                            Expression expression = (Expression) obj;
-                            String text = expression.evaluate(exchange, 
String.class);
-                            if (text != null) {
-                                buffer.append(text);
-                            }
-                        } else {
-                            buffer.append((String) obj);
-                        }
+                for (Expression expression : expressions) {
+                    String text = expression.evaluate(exchange, String.class);
+                    if (text != null) {
+                        buffer.append(text);
                     }
+                }
+                return buffer.toString();
+            }
+
+            @Override
+            public void init(CamelContext context) {
+                boolean constant = false;
+                for (Expression expression : expressions) {
+                    expression.init(context);
+                }
+            }
+
+            @Override
+            public String toString() {
+                if (description != null) {
+                    return description;
                 } else {
-                    for (Expression expression : expressions) {
+                    return "concat(" + expressions + ")";
+                }
+            }
+        };
+    }
+
+    /**
+     * Returns an optimized expression which returns the string concatenation 
value of the various.
+     * expressions
+     *
+     * @param context the Camel context
+     * @param expressions the expression to be concatenated dynamically
+     * @param description the text description of the expression
+     * @return an expression which when evaluated will return the concatenated 
values
+     */
+    private static Expression concatExpressionOptimized(final CamelContext 
context, final Collection<Expression> expressions, final String description) {
+        Collection<Object> preprocessedExpression = new 
ArrayList<>(expressions.size());
+        for (Expression expression : expressions) {
+            if (expression instanceof ConstantExpressionAdapter) {
+                expression.init(context);

Review Comment:
   I did not realize that for some `ConstantExpressionAdapter` we had to 
initialize it first to get its value, I don't know if it is acceptable to call 
`init` so early? @oscerd WDYT?
   
   If it is not acceptable, the other way could be to use a volatile boolean 
field to know if the expression has been initialized or not and use the 
Double-checked locking approach to ensure that it is initialized once. Even if 
it adds a locking mechanism it is only during the initialization phase which 
happens only once per expression when properly used so the overhead is limited.



##########
core/camel-support/src/main/java/org/apache/camel/support/builder/ExpressionBuilder.java:
##########
@@ -1577,74 +1578,122 @@ public String toString() {
      * Returns an expression which returns the string concatenation value of 
the various
      * expressions
      *
+     * @param context the Camel context
      * @param expressions the expression to be concatenated dynamically
      * @return an expression which when evaluated will return the concatenated 
values
      */
-    public static Expression concatExpression(final Collection<Expression> 
expressions) {
-        return concatExpression(expressions, null);
+    public static Expression concatExpression(final CamelContext context, 
final Collection<Expression> expressions) {
+        return concatExpression(context, expressions, null);
     }
 
     /**
      * Returns an expression which returns the string concatenation value of 
the various
      * expressions
      *
+     * @param context the Camel context
      * @param expressions the expression to be concatenated dynamically
      * @param description the text description of the expression
      * @return an expression which when evaluated will return the concatenated 
values
      */
-    public static Expression concatExpression(final Collection<Expression> 
expressions, final String description) {
-        return new ExpressionAdapter() {
+    public static Expression concatExpression(final CamelContext context, 
final Collection<Expression> expressions, final String description) {
+
+        for (Expression expression : expressions) {
+            if(expression instanceof ConstantExpressionAdapter){
+                return concatExpressionOptimized(context, expressions, 
description);
+            }
+        }
 
-            private Collection<Object> col;
+        return concatExpression(expressions,description);
+    }
+
+    /**
+     * Returns an expression which returns the string concatenation value of 
the various
+     * expressions
+     *
+     * @param expressions the expression to be concatenated dynamically
+     * @param description the text description of the expression
+     * @return an expression which when evaluated will return the concatenated 
values
+     */
+    private static Expression concatExpression(final Collection<Expression> 
expressions, final String description) {
+        return new ExpressionAdapter() {
 
             @Override
             public Object evaluate(Exchange exchange) {
                 StringBuilder buffer = new StringBuilder();
-                if (col != null) {
-                    // optimize for constant expressions so we can do this a 
bit faster
-                    for (Object obj : col) {
-                        if (obj instanceof Expression) {
-                            Expression expression = (Expression) obj;
-                            String text = expression.evaluate(exchange, 
String.class);
-                            if (text != null) {
-                                buffer.append(text);
-                            }
-                        } else {
-                            buffer.append((String) obj);
-                        }
+                for (Expression expression : expressions) {
+                    String text = expression.evaluate(exchange, String.class);
+                    if (text != null) {
+                        buffer.append(text);
                     }
+                }
+                return buffer.toString();
+            }
+
+            @Override
+            public void init(CamelContext context) {
+                boolean constant = false;
+                for (Expression expression : expressions) {
+                    expression.init(context);
+                }
+            }
+
+            @Override
+            public String toString() {
+                if (description != null) {
+                    return description;
                 } else {
-                    for (Expression expression : expressions) {
+                    return "concat(" + expressions + ")";
+                }
+            }
+        };
+    }
+
+    /**
+     * Returns an optimized expression which returns the string concatenation 
value of the various.
+     * expressions
+     *
+     * @param context the Camel context
+     * @param expressions the expression to be concatenated dynamically
+     * @param description the text description of the expression
+     * @return an expression which when evaluated will return the concatenated 
values
+     */
+    private static Expression concatExpressionOptimized(final CamelContext 
context, final Collection<Expression> expressions, final String description) {
+        Collection<Object> preprocessedExpression = new 
ArrayList<>(expressions.size());
+        for (Expression expression : expressions) {
+            if (expression instanceof ConstantExpressionAdapter) {
+                expression.init(context);
+                Object value = ((ConstantExpressionAdapter) 
expression).getValue();
+                preprocessedExpression.add(value.toString());
+            } else {
+                preprocessedExpression.add(expression);
+            }
+        }
+
+        return new ExpressionAdapter() {
+
+            private final Collection<Object> col = 
Collections.unmodifiableCollection(preprocessedExpression);
+
+            @Override
+            public Object evaluate(Exchange exchange) {
+                StringBuilder buffer = new StringBuilder();
+                for (Object obj : col) {
+                    if (obj instanceof Expression) {
+                        Expression expression = (Expression) obj;
                         String text = expression.evaluate(exchange, 
String.class);
                         if (text != null) {
                             buffer.append(text);
                         }
+                    } else {
+                        buffer.append((String) obj);
                     }
                 }
                 return buffer.toString();
             }
 
             @Override
             public void init(CamelContext context) {
-                boolean constant = false;
                 for (Expression expression : expressions) {
                     expression.init(context);

Review Comment:
   Assuming that it is acceptable to call `init` when creating the expression, 
we could avoid calling init on expressions of type `ConstantExpressionAdapter` 
as they are already initialized



-- 
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: commits-unsubscr...@camel.apache.org

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

Reply via email to