[jboss-jira] [JBoss JIRA] (DROOLS-4296) Rewrite parsing of modified properties

[Toshiya Kobayashi (Jira)]

    [ https://issues.redhat.com/browse/DROOLS-4296?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13825111#comment-13825111 ] 

Toshiya Kobayashi commented on DROOLS-4296:
-------------------------------------------

Talked with Luca.

Case A) 
{noformat}
{ modify($p) { age = $p.age+1 }; }
{noformat}
with mvel dialect

  -> The expression is parsed to an AssignExpr inside a ModifyStatement. AssignExpr.getTarget() returns "age" so we can store such information and skip findModifiedProperties().

Case B)
{noformat}
{ modify($p) { setName($p.getName() + "30"); }
{noformat}
with mvel/java dialect

  -> The expression is parsed to an ExpressionStmt inside a ModifyStatement. We cannot get "age" directly from the ExpressionStmt. So we will use findModifiedProperties() later.

Even if we add logic to store modified properties for the case A), we still keep findModifiedProperties() for case B). Also we would need to consider the case where a user mixes the expressions with mvel dialect. e.g. 
{noformat}
{ modify($p) { age = $p.age+1, setName($p.getName() + "30"); }
{noformat}

The current implementation of findModifiedProperties() doesn't fully parse the modify block. It iterates MethodCallExpr so I think it's not inefficient. Rather, it's good to have one place to process for all the cases.

https://github.com/kiegroup/drools/blob/master/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/Consequence.java#L328-L345

We concluded that we don't need to refactor it.

> Rewrite parsing of modified properties
> --------------------------------------
>
>                 Key: DROOLS-4296
>                 URL: https://issues.redhat.com/browse/DROOLS-4296
>             Project: Drools
>          Issue Type: Enhancement
>            Reporter: Luca Molteni
>            Assignee: Toshiya Kobayashi
>            Priority: Minor
>              Labels: good-first-issue
>
> See Consequence::findModifiedProperties
> Currently modified properties are parsed throughout the consequence, but that information is already present after the preprocessing of the ModifyStatement so it can be simplified a lot.
> This can be forward ported to Kogito also
>  



--
This message was sent by Atlassian Jira
(v7.13.8#713008)