gnodet-bot commented on code in PR #26806:
URL: https://github.com/apache/camel/pull/26806#discussion_r4085380200
##########
core/camel-core-languages/src/main/java/org/apache/camel/language/simple/ast/LogicalExpression.java:
##########
@@ -93,40 +93,22 @@ public Expression createExpression(CamelContext
camelContext, String expression)
final Expression leftExp = left.createExpression(camelContext,
expression);
final Expression rightExp = right.createExpression(camelContext,
expression);
+ // build the predicate once, not for every message
+ final Predicate leftPredicate =
ExpressionToPredicateAdapter.toPredicate(leftExp);
+ final Predicate rightPredicate =
ExpressionToPredicateAdapter.toPredicate(rightExp);
if (operator == LogicalOperatorType.AND) {
- return createAndExpression(leftExp, rightExp);
+ return createExpression(PredicateBuilder.and(leftPredicate,
rightPredicate));
} else if (operator == LogicalOperatorType.OR) {
- return createOrExpression(leftExp, rightExp);
+ return createExpression(PredicateBuilder.or(leftPredicate,
rightPredicate));
}
throw new SimpleParserException("Unknown logical operator " +
operator, token.getIndex());
}
- private Expression createAndExpression(final Expression leftExp, final
Expression rightExp) {
+ private Expression createExpression(final Predicate predicate) {
return new Expression() {
@Override
public <T> T evaluate(Exchange exchange, Class<T> type) {
- Predicate predicate =
ExpressionToPredicateAdapter.toPredicate(leftExp);
- predicate = PredicateBuilder.and(predicate,
ExpressionToPredicateAdapter.toPredicate(rightExp));
-
- boolean answer = predicate.matches(exchange);
- return
exchange.getContext().getTypeConverter().convertTo(type, answer);
- }
-
- @Override
- public String toString() {
- return left + " " + token.getText() + " " + right;
- }
- };
- }
-
- private Expression createOrExpression(final Expression leftExp, final
Expression rightExp) {
- return new Expression() {
- @Override
- public <T> T evaluate(Exchange exchange, Class<T> type) {
- Predicate predicate =
ExpressionToPredicateAdapter.toPredicate(leftExp);
- predicate = PredicateBuilder.or(predicate,
ExpressionToPredicateAdapter.toPredicate(rightExp));
-
boolean answer = predicate.matches(exchange);
return
exchange.getContext().getTypeConverter().convertTo(type, answer);
}
Review Comment:
⚠️ **Missing `toString()` on the new shared `createExpression(Predicate)`
method.**
Both `createAndExpression` and `createOrExpression` previously overrode
`toString()` on the anonymous `Expression` they returned:
```java
@Override
public String toString() {
return left + " " + token.getText() + " " + right;
}
```
The new unified `createExpression(Predicate)` dropped that override.
Production log lines and exception messages will now show
`LogicalExpression$1@xxxxxx` instead of the readable `${body} && ${header.x} >
0` form, making debugging significantly harder.
Please add the `toString()` override back to the anonymous class, closing
over `left`, `token`, and `right`.
##########
core/camel-core-languages/src/main/java/org/apache/camel/language/simple/ast/OtherExpression.java:
##########
@@ -109,4 +110,9 @@ public String toString() {
}
};
}
+
+ private static boolean isZero(Object value) {
+ // any kind of number such as 0, 0L, 0.0 or BigDecimal.ZERO
+ return value instanceof Number n && n.doubleValue() == 0;
Review Comment:
🔍 **`isZero(-0.0)` is `true` — document intent.**
`-0.0 == 0.0` is `true` in IEEE 754, so `isZero(-0.0)` returns `true`,
meaning negative zero is treated as falsy by `?:`. This is probably the right
call, but it is a non-obvious edge case worth a comment:
```suggestion
return value instanceof Number n && n.doubleValue() == 0; // -0.0 ==
0.0 in IEEE 754, so -0.0 is also falsy; NaN is NOT zero and is truthy
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
