yesamer commented on code in PR #6213:
URL:
https://github.com/apache/incubator-kie-drools/pull/6213#discussion_r1922089905
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/impl/RangeImpl.java:
##########
@@ -97,32 +98,33 @@ public boolean isWithUndefined() {
return withUndefined;
}
- private Boolean finiteRangeIncludes(Object param) {
+ private Boolean finiteRangeIncludes(FEELDialect feelDialect, Object param)
{
+ // Defaulting FEELDialect to FEEL
Review Comment:
@gitgabrio This comment it's a bit misleading to me, where FEELDialect is
set to FEEL as default?
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/FEELFunction.java:
##########
@@ -70,6 +70,24 @@ public interface FEELFunction {
*/
Object invokeReflectively(EvaluationContext ctx, Object[] params);
+ /**
+ * The default value to return instead of <code>null</code>, to be used
with the B-FEEL (and other different dialects) syntax
+ * @return
+ */
+ default Object defaultValue() {
+ // To be overridden by specific classes for B-FEEL compliance
+ return null;
+ }
Review Comment:
@gitgabrio Considering the vaule returned by `defaultValue` is used in a
very specific scope, that is when B-FEEL is enabled (according to the usage in
the BaseFEELFunction class), I would consider removing the default behavior of
this method in FeelFunction interface. In this way, we will have the
opportunity to discover at compile-time if we forgot to correctly manage a
specific Function with B-FEEL and we'll remove any chance of returning null in
such a case. Eg. if with the current behavior, we forget to manage a specific
function overriding that method, with this default logic we will return null as
a default value, that is conflicting with the B-FEEL specs that should never
return null. If I wasn't clear enough, please reach me in private
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/functions/MatchesFunction.java:
##########
@@ -57,4 +58,9 @@ public FEELFnResult<Boolean> invoke(@ParameterName("input")
String input, @Param
return FEELFnResult.ofError(new
InvalidParametersEvent(Severity.ERROR, errorMessage, e));
}
}
+
+ @Override
+ public Object defaultValue() {
+ return false;
+ }
Review Comment:
@gitgabrio I guess this is override is not needed, as it duplicates the same
behavior of the superclass's logic.
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/functions/BaseFEELFunction.java:
##########
@@ -80,6 +81,14 @@ public Object invokeReflectively(EvaluationContext ctx,
Object[] params) {
CandidateMethod cm = getCandidateMethod(ctx, params,
isNamedParams);
if (cm != null) {
+ if (cm.actualParams != null) {
+ for (int i = 0; i < cm.actualParams.length; i++) {
+ if (cm.actualParams[i] instanceof List list) {
+ cm.actualParams[i] =
getFEELDialectAdaptedList(ctx, list);
+ }
+ }
+ }
Review Comment:
@gitgabrio IIUC, the function parameters are coerced based on the FEEL
dialect, right?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
