Re: [PR] Return coerced null from decision with knowledge requirements [incubator-kie-drools]

["baldimir (via GitHub)" <gi...@apache.org>]

baldimir commented on code in PR #6408:
URL: https://github.com/apache/incubator-kie-drools/pull/6408#discussion_r2245034160


##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNRuntimeImpl.java:
##########
@@ -455,7 +455,7 @@ public static Object coerceUsingType(Object value, DMNType type, boolean typeChe
         if (!typeCheck) {
             return result;
         }
-        if (type.isAssignableValue(result)) {
+        if (type.isAssignableValue(value)) {

Review Comment:
   I think this whole method looks a bit cumbersome (not because of the change, it just builds on top of what is already there). It returns on various places and it is not visible on first sight what it is doing. Maybe we could replace it with something like this: 
   
   ```
   public static Object coerceUsingType(Object value, DMNType type, boolean typeCheck, BiConsumer<Object, DMNType> nullCallback) {
           if (typeCheck) {
               if (type.isAssignableValue(value)) {
                   return coerceSingleItemCollectionToValue(value, type);
               } else {
                   nullCallback.accept(value, type);
                   return null;
               }
           } else {
               return coerceSingleItemCollectionToValue(value, type);
           }
       }
   
       /**
        * Checks a type and if it is not a collection type, checks if the specified value is a collection 
        * that contains only a single value and if yes, coerces the collection to the single item itself.  
        * E.g. [1] becomes 1. Basically it unwraps the single item from a collection, if it is required. 
        * 
        * @param value Value that is checked and potentially coerced to a single item.  
        * @param type Required type. Based on this type, it is determined, if the coercion happens. 
        *             If the requirement is for a non-collection type and the value is a single item collection, 
        *             the coercion happens. 
        * @return If all requirements are met, returns coerced value. Otherwise returns the original value. 
        */
       private static Object coerceSingleItemCollectionToValue(Object value, DMNType type) {
           if (!type.isCollection() && value instanceof Collection && ((Collection<?>) value).size() == 1) {
               // as per Decision evaluation result.
               return ((Collection<?>) value).toArray()[0];
           } else {
               return value;
           }
       }
   ```
   What do you think, please? 



-- 
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-unsubscribe@kie.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org