[jira] Created: (OFBIZ-3774) Add The Visitor Pattern To Screen Widget Model Classes

[Nicolas Malin (Jira)]

Add The Visitor Pattern To Screen Widget Model Classes
------------------------------------------------------

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


From time to time there has been interest expressed in introducing the visitor pattern to the screen widget model classes. This issue is intended to provide a forum on the subject and a means to vote on it. Details are in the comments.


--
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-3774?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adrian Crum updated OFBIZ-3774:
-------------------------------

    Attachment: OFBIZ-3774.patch

The attached patch is a beginning step - it has the visitor interface for the screen sub-widgets, and it adds the necessary accept method to the sub-widgets. There is no interface implementation.

The screen widget model classes already use a pattern similar to visitor called double dispatch. Essentially, the ScreenStringRenderer implementation calls the renderWidgetString method of each widget, and that widget method calls the renderXxxx method in the ScreenStringRenderer. With the visitor pattern, the renderWidgetString method is called accept, and the renderXxxx method is called visit. So, it's basically the same design but with a slight difference.

In the current code, the visitor (ScreenStringRenderer) passes a number of arguments to the model widget renderWidgetString method. In the attached patch, the visitor (ScreenWidgetVisitor) passes only itself as an argument to the model widget accept method. The reason for this difference is flexibility and expandability. Set up this way, the visitor could be anything - a renderer, an artifact info gatherer, a pretty printer, etc. There is no implementation implied in the design.

Each renderer would implement ScreenWidgetVisitor. The code inside each model widget renderWidgetString method would be moved to the corresponding visit method in the renderer. Likewise, the artifact-info-gatherer would implement ScreenWidgetVisitor. The existing model widget artifact-info-gathering code would be moved to the corresponding visit method in the artifact-info-gatherer.

Once those changes are made, the model widgets will contain only object construction code. Even that code could be moved to a builder class that implements ScreenWidgetVisitor.

The benefit of this change will be improved separation of concerns. Right now the screen widget model classes try to be everything to everybody. After the change they will be little more than data structures that various behavior-specific objects will refer to. Rendering code is not mixed in with object construction code or with artifact-gathering code. The API is simplified.

A change like this could have serious downstream consequences for OFBiz users who have modified the screen widget model classes. One approach we could take is to put the visit methods in ScreenStringRenderer (instead of having a separate interface) and then deprecate the existing renderXxxx methods. The downside is code maintainance - bug fixes and new features will have to be applied to both patterns.

If you believe the change is worthwhile, then please vote on this issue.

> Add The Visitor Pattern To Screen Widget Model Classes
> ------------------------------------------------------
>
>                 Key: OFBIZ-3774
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-3774
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: OFBIZ-3774.patch
>
>
> From time to time there has been interest expressed in introducing the visitor pattern to the screen widget model classes. This issue is intended to provide a forum on the subject and a means to vote on it. Details are in the comments.

--
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-3774?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12868470#action_12868470 ]

Marc Morin commented on OFBIZ-3774:
-----------------------------------

We did a similar visitor pattern to the entire presentment (Screen, Form, Menu) space and the controller.  This path, mainly separation of concern, is very powerful, and allows easy extension to the system.

The typical implementation path that has been taken, is to develop a series of abstract visitors that have a few base behaviors, navigation of the components, and throwing exceptions for each component.  This enables a specific visitors to extend the base navigation class, to get full model navigation and add any specific behavior with a surprisingly small amount of code.

We've done "Presentment" visitors for walking, validation (examining location/name references looking for dangling screen/from/controller references, etc.. used in junit test cases - fail build if broken links),  security auto-notation (propagates service security declarations, to screens model objects, and will automatically remove buttons, links to areas in the application that the user doesn't have rights to access.)  We have a partial dump (pretty-print) for the PresentmentModel as well.

Externalizing the creation of the model objects, is crucial to getting the full power of this type of change. The model objects should have nothing but their attributes with their getter/setter methods (and the accept).

> Add The Visitor Pattern To Screen Widget Model Classes
> ------------------------------------------------------
>
>                 Key: OFBIZ-3774
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-3774
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: OFBIZ-3774.patch
>
>
> From time to time there has been interest expressed in introducing the visitor pattern to the screen widget model classes. This issue is intended to provide a forum on the subject and a means to vote on it. Details are in the comments.

--
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-3774?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12868503#action_12868503 ]

Adrian Crum commented on OFBIZ-3774:
------------------------------------

One thing I would like to clarify: the patch only includes screen sub-widgets, but the goal would be apply the visitor pattern to all screen widget model classes - like Marc mentioned.

An abstract class for renderers that navigates the model structure probably won't work - because each rendering format will require its own sequence of events, or its own mode of navigation. You can't determine in advance how a particular rendering format will need to navigate the structure. The existing widget models try to do this form of prediction and it has caused problems in the past.

I'm currently working on porting the macro renderer over to the visitor interface to see what kind of problems we might encounter.

> Add The Visitor Pattern To Screen Widget Model Classes
> ------------------------------------------------------
>
>                 Key: OFBIZ-3774
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-3774
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: OFBIZ-3774.patch
>
>
> From time to time there has been interest expressed in introducing the visitor pattern to the screen widget model classes. This issue is intended to provide a forum on the subject and a means to vote on it. Details are in the comments.

--
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-3774?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adrian Crum updated OFBIZ-3774:
-------------------------------

    Attachment: OFBIZ-3774.patch

Updated patch. I implemented a visitor renderer by copying the MacroScreenRenderer code.

Making the change wasn't too hard. The visitor pattern actually makes a lot of things easier from the rendering viewpoint. The difficulties I ran into were due to maintaining backward-compatibility.

> Add The Visitor Pattern To Screen Widget Model Classes
> ------------------------------------------------------
>
>                 Key: OFBIZ-3774
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-3774
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: OFBIZ-3774.patch, OFBIZ-3774.patch
>
>
> From time to time there has been interest expressed in introducing the visitor pattern to the screen widget model classes. This issue is intended to provide a forum on the subject and a means to vote on it. Details are in the comments.

--
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-3774?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adrian Crum updated OFBIZ-3774:
-------------------------------

    Attachment: OFBIZ-3774.patch

Updated patch to latest trunk revision.

> Add The Visitor Pattern To Screen Widget Model Classes
> ------------------------------------------------------
>
>                 Key: OFBIZ-3774
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-3774
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>            Reporter: Adrian Crum
>            Priority: Minor
>         Attachments: OFBIZ-3774.patch, OFBIZ-3774.patch, OFBIZ-3774.patch
>
>
> From time to time there has been interest expressed in introducing the visitor pattern to the screen widget model classes. This issue is intended to provide a forum on the subject and a means to vote on it. Details are in the comments.

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