[jira] Created: (OFBIZ-1924) Sandbox: Improved FlexibleStringExpander class

[Nicolas Malin (Jira)]

Sandbox: Improved FlexibleStringExpander class
----------------------------------------------

                 Key: OFBIZ-1924
                 URL: https://issues.apache.org/jira/browse/OFBIZ-1924
             Project: OFBiz
          Issue Type: Improvement
          Components: framework
            Reporter: Adrian Crum
            Priority: Minor


As discussed on the dev mailing list, implement an improved FlexibleStringExpander that uses a factory method to create class instances.


--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-1924?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adrian Crum updated OFBIZ-1924:
-------------------------------

    Attachment: FlexibleStringExpander.patch

FlexibleStringExpander.patch contains the improved class. It has been refactored to construct a better parse tree and to eliminate redundant code.

This needs to be tested thoroughly before committing.

> Sandbox: Improved FlexibleStringExpander class
> ----------------------------------------------
>
>                 Key: OFBIZ-1924
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-1924
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: FlexibleStringExpander.patch
>
>
> As discussed on the dev mailing list, implement an improved FlexibleStringExpander that uses a factory method to create class instances.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-1924?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adrian Crum updated OFBIZ-1924:
-------------------------------

    Attachment: FlexibleStringExpander.patch

Updated patch - final version.

> Sandbox: Improved FlexibleStringExpander class
> ----------------------------------------------
>
>                 Key: OFBIZ-1924
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-1924
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: FlexibleStringExpander.patch, FlexibleStringExpander.patch
>
>
> As discussed on the dev mailing list, implement an improved FlexibleStringExpander that uses a factory method to create class instances.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-1924?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adrian Crum updated OFBIZ-1924:
-------------------------------

    Attachment: FlexibleStringExpander_Complete_Refactor.patch

FlexibleStringExpander_Complete_Refactor.patch contains the entire refactor.

1. All "new FlexibleStringExpander(...)" occurrences have been replaced with the factory method.
2. The refactored FlexibleStringExpander class has been tested - it seems to be working well.

I decided to cache only the expressions that need expanding. I simplified the class enough that a FlexibleStringExpander instance that only wraps a String doesn't create much overhead.

If anyone wants to test this, that would be great. Otherwise I'll wait a few days, then commit it.

> Sandbox: Improved FlexibleStringExpander class
> ----------------------------------------------
>
>                 Key: OFBIZ-1924
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-1924
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: FlexibleStringExpander.patch, FlexibleStringExpander.patch, FlexibleStringExpander_Complete_Refactor.patch
>
>
> As discussed on the dev mailing list, implement an improved FlexibleStringExpander that uses a factory method to create class instances.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-1924?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12623826#action_12623826 ]

David E. Jones commented on OFBIZ-1924:
---------------------------------------

Quick note on caching only the expressions that need expanding: even if it doesn't need expanding it does create additional objects and those add up, so it may be well worth doing.

> Sandbox: Improved FlexibleStringExpander class
> ----------------------------------------------
>
>                 Key: OFBIZ-1924
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-1924
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: FlexibleStringExpander.patch, FlexibleStringExpander.patch, FlexibleStringExpander_Complete_Refactor.patch
>
>
> As discussed on the dev mailing list, implement an improved FlexibleStringExpander that uses a factory method to create class instances.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-1924?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12623833#action_12623833 ]

Adrian Crum commented on OFBIZ-1924:
------------------------------------

Thanks for the advice David. I checked out the cache contents in WebTools and saw cache keys like:

0
1
Y
N
true
false

and it just seemed silly to cache everything. I'd like to commit it as-is. If someone feels strongly otherwise, it can be changed by removing a few lines.

We will still save a lot of memory because empty expressions are sharing a common class instance.

> Sandbox: Improved FlexibleStringExpander class
> ----------------------------------------------
>
>                 Key: OFBIZ-1924
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-1924
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: FlexibleStringExpander.patch, FlexibleStringExpander.patch, FlexibleStringExpander_Complete_Refactor.patch
>
>
> As discussed on the dev mailing list, implement an improved FlexibleStringExpander that uses a factory method to create class instances.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-1924?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adrian Crum closed OFBIZ-1924.
------------------------------

    Resolution: Fixed

Fixed, rev 687422.

> Sandbox: Improved FlexibleStringExpander class
> ----------------------------------------------
>
>                 Key: OFBIZ-1924
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-1924
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: FlexibleStringExpander.patch, FlexibleStringExpander.patch, FlexibleStringExpander_Complete_Refactor.patch
>
>
> As discussed on the dev mailing list, implement an improved FlexibleStringExpander that uses a factory method to create class instances.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.