Hello Daniel, I'm a little confuse to take more than 6 month to response.
You push me on my currently java knowledge limit, and after analyze I didn't understand all you done. However, your works feel good. During my works on the OFBIZ-11810 [1], I confronted to many duplicated code between Screen, Form and Menu render. This case help me to understand an advantage of your refacto. I'm clearly in favor to continue the refactoring, and I have a question. Because we works on the same code what do you prefer ? * I commit my work with the current code and we migrate it after * We migrate before the code and I update my work on it * I try to convert my current improvement with your refactoring proposal. Nicolas [1] https://issues.apache.org/jira/browse/OFBIZ-11810 On 28/07/2020 19:49, Daniel Watford wrote: > Hello, > > OFBIZ-11900 covers the work-in-progress effort to refactor > MacroFormRenderer by extracting code which builds strings to be executed as > FTL macros into a separate builder class. > > This github branch [1] contains the work-in-progress changes for this > issue. Please take a look at this branch and highlight any concerns with > the approach. This will give me a chance to address concerns or change > approach before too much further work is done. > > Further to simply extracting code from MacroFormRenderer to a separate > builder I wanted to avoid the use of strings to represent FTL macro calls. > Instead I added an interface called RenderableFtl to represent an element > that can be rendered as part of a FreeMarker template. > > So far the main RenderableFtls in use are RenderableFtlMacroCall and > RenderableFtlString. > > RenderableFtlString can be used to represent a pre-rendered HTML string as > used to construct hidden forms by WidgetWorker. > > RenderableFtlMacroCall is probably the most important of the RenderableFtl > classes as it can be used in place of the strings previously built in > MacroFormRenderer to capture an FTL macro call, but without the need for > the caller to append quoting and whitespace delimiters for each additional > macro call parameter. > > All the RenderableFtl classes know how to render themselves to a String > which will be used by FtlWriter to process the string as an FTL element and > then write the result to an Appendable. > > The reason for these changes are to separate the various concerns involved > in rendering a form: > - RenderableFtl objects are responsible for rendering their data to an FTL > compliant string. > - FtlWriter is responsible for processing FTL compliant strings as part of > an FTL template and writing the results to an Appendable. > - RenderableFtlFormElementsBuilder is responsible for creating > RenderableFtl objects for various FieldInfo objects, replacing the FTL > macro call strings which were previously built in MacroFormRenderer. > - MacroFormRenderer is responsible for orchestrating calls to > RenderableFtlFormElementsBuilder to create multiple RenderableFtl objects > as needed for rendering a field and passing them to FtlWriter. For example, > rendering a DisplayEntityField produces RenderableFtl objects representing > the displayed field, an associated hyperlink and a tooltip. > > Separating these concerns makes testing easier: > - MacroFormRendererTest can test orchestration of MacroFormRender's calls > to RenderableFtlFormElementsBuilder, rather than having to analyse the > strings previously written. > - Testing of RenderableFtlFormElementsBuilder focuses on the production of > correctly configured RenderableFtl elements based on the given FieldInfo > objects, rather than testing the eventual string rendered. > - The RenderableFtl objects can be tested by examining the FTL strings > produced. > > To support testing of RenderableFtlFormElementsBuilder a number of Hamcrest > Matchers have been created to check that RenderableFtlMacroCall objects > have expected name and parameter values. > > Please excuse the long message :) and please let me know if you have any > comments or concerns about the approach I have taken. > > Thanks, > > Dan. > > [1] - https://github.com/danwatford/ofbiz-framework/tree/OFBIZ-11900-WIP >
