Thanks Daniel,From what I see in your patch, and the completing information you gave us below (thank!), I don't see much troubles (if any) to adopt Lombok, so +1 from my side
Jacques Le 26/08/2020 à 10:56, Daniel Watford a écrit :
Hello, A pull request [1] has been created for OFBIZ-11900 which makes use of some Lombok annotations. It is very close to the WIP branch posted in my original message to this thread. Jacques has taken a quick look but quite rightly reminded me that we didn't really conclude the Lombok discussion, therefore I'd like to bring up the discussion again before we decide how to proceed with the pull request. Girish, the Lombok changelog [2] indicates support for JDK 14. I don't think we need to worry about our compiler version not being supported. I would expect the Lombok Project to only declare support for a JDK version once they have proven all stable features are working correctly. If there were any problems then there are many users of the project that would be very keen to restore correctly functionality. Since OFBiz is currently constrained to JDK 8 I'm happy to assume Lombok will have full support for whatever JDK we eventually move to. There are no guarantees here, but the risk seems very small to me. Remember if we had a problem using some annotations before upgrading to a new JDK then we could run de-lombok to generate vanilla java sources from that point on. Adding an additional tool will likely add to the project build time and memory requirements. There is a cost to running tools such as lombok, checkstyle, findbugs, junit, etc. In my opinion the benefits outweigh the costs. In the stackoverflow post [3] cited the Lombok team highlighted improvements that reduced build time in subsequent releases of the lombok project. The OP also stated (after the higher-performant release was available) that there was little difference between compiling de-lomboked code vs the code with lombok annotations. As far as which features of Lombok we choose to accept, I'd recommend excluding the experimental features. All stable features should be considered acceptable, but the committer will have final say on a case-by-case basis. The committer will need to determine whether the use of Lombok for a particular case improves code readability and maintainability long-term. Michael, based on the video [4] mentioned previously and this discussion I think I'd summarise the PROS and CONS of OFBiz using Lombok as: PRO - Implement patterns without boilerplate code. - Best-practice implementations for common methods such as equals and hashCode. - Less code is easier to maintain and reason about. - Runtime behaviour is unchanged. - Lombok annotations can be removed in future if needed by 'de-lomboking'. CON - Another tool in the build pipeline. - Learning curve for developers. - Risk that code might not compile against future JDK upgrades. - IDE plugin required. (You can get away without it but the development experience is diminished) All, please come back with whether you are for or against use of lombok in OFBiz or whether you need more information. If further evaluation is needed I'm happy to dig into this. But I would need some guidance as to what questions needed to be answered and what would be considered an acceptable method to find those answers? E.g. measuring compilation time in vanilla code vs lombok annotated code? Measuring lines-of-code differences between annotated and vanilla code for the same functionality? etc. Thanks, Dan. [1] - https://github.com/apache/ofbiz-framework/pull/232 [2] - https://projectlombok.org/changelog [3] - https://stackoverflow.com/questions/15518405/lombok-slowing-down-build-process-in-large-project#:~:text=Lombok%20is%20an%20annotation%20processor,sources%20or%20throw%20compiler%20errors . [4] - https://projectlombok.org/ On Tue, 28 Jul 2020 at 15:53, Daniel Watford <d...@foomoo.co.uk> wrote:Hi Michael, I'd encourage tool use to help us produce software wherever possible, particularly those which are common in the java ecosystem and have a low barrier to entry. In my opinion Lombok is common enough that there are many tutorials available to help developers get up to speed, and it only has an effect on code which is explicitly annotated to use its features so won't have an impact on existing software. Please take a look at the 4 minute video on the Lombok home page [1] where they give a very nice demonstration of how lombok helps reduce boilerplate code. They also link to an article [2] from 2010 about using lombok to reduce boilerplate. There are sections on Limitations and Controversy in the article. I raised the memory issue in case it would be a problem for the build infrastructure, but it doesn't have any impact on runtime performance. Lombok helps us reduce boilerplate code. Rather than maintaining such code by hand we use a tool to do it for us instead, allowing us to reason about code in terms of patterns rather than low-level implementation. For example, if I see that a class is annotated with @Value then I know instances of that class are immutable without needing to double check all the fields are final and initialised in the constructor. If I see a class annotated with @Builder then I know the builder pattern has been applied and the author has taken steps to ensure all elements/configuration needed to construct a valid instance of the class are set. The author can do this by overriding the build method of the Builder class if the default lombok code doesn't satisfy their requirements. Applying the (experimental) @UtilityClass annotation to a class will ensure the default constructor is private rather than having to write the constructor explicitly. I think the above advantages outweigh the disadvantages of introducing an additional ofbiz build step (as would be done by the gradle plugin). I did experiment within IntelliJ by removing the IDE plugin. I could still work on code that made use of lombok annotated classes, but the experience was significantly diminished. IntelliJ still uses gradle for compilation, so I didn't have any problems there, but all the IDE support for code assistance was lost for those classes. I would recommend developers install tools relevant to their IDE if working with lombok annotated classes, but it is not a requirement. There will be a learning curve for developers using lombok, but if we start small we will have patterns we can refer new developers to. Also if it was deemed inappropriate for a particular class to make use of lombok annotations then we could implement the relevant code ourselves. [1] - https://projectlombok.org/ [2] - https://objectcomputing.com/resources/publications/sett/january-2010-reducing-boilerplate-code-with-project-lombok On Tue, 28 Jul 2020 at 14:10, Michael Brohl <michael.br...@ecomify.de> wrote:Hi Daniel, can you explain what the advantages and disadvantages are? Is it worth the introduction of an additional framework, more complex IDE configuration, an additional Gradle plugin and more memory consumption and why? Thanks for clarification, Michael Brohl ecomify GmbH - www.ecomify.de Am 28.07.20 um 14:44 schrieb Daniel Watford:Hello, Back in April the possibility of using Lombok for the generation of some boilerplate code was mentioned on the mailing list [1]. As part of work-in-progress on OFBIZ-11900 (refactoringMacroFormRenderer)I have used Lombok on a few small classes. The work-in-progress branchcanbe found at [2]. Only a small amount of Lombok has been used so far, meaning itshouldn't betoo difficult to remove it if needed. In build.gradle I have used the FreeFair Gradle Lombok plugin [3] referenced by the Lombok Project [4]. Building with the lombok plugin seemed to use a lot of memory and caused gradle to garbage collect and run out of heap regularly. To resolvethis Iincreased the about of memory available to gradle in gradle.properties using: org.gradle.jvmargs=-Xmx2g -XX\:MaxHeapSize\=4g Tuning this might be important depending on the CI infrastructure usedbythe Ofbiz project. You will likely need your IDE to apply annotation processing otherwiseyoumight see warning of missing methods. In IntelliJ I use what appears tobethe de facto Lombok plugin [5]. Guidance is available from the Lombok project for other IDEs [6]. Lombok @ToString annotations have been applied to RenderableFtlStringandRenderableFtlNoop. This causes Lombok to insert toString() methods intotheclasses based on the class names and field values. The @Value annotation has been applied to MacroCallParameterStringValue, MacroCallParameterBooleanValue and MacroCallParameterMapValue. This annotation turns those classes into immutable-like entities, where all fields must be set in the inserted constructor and are available from automatically inserted getters. ToString(), equals and hashCode() methods are also created meaning these classes can be relied upon as map keys if needed. The @Builder annotation has been applied to RenderableFtlMacroCall and RenderableFtlSequence. This annotation does quite a lot so I'd recommend you run delombok (instructions below) to see the code thatLombokinserts for us. To see the sources generated by Lombok we can run DeLombok. At thecommandline execute: ./gradlew delombok A copy of all sources (not just those with lombok annotations) will be placed under build/delombok. Please take a look at the delombokedsourcesfor the above classes under build/delombok/main/org/apache/ofbiz/widget/renderer/macro/parameter and build/delombok/main/org/apache/ofbiz/widget/renderer/macro/renderable. Please let me know what you think about this usage of Lombok. Thanks, Dan. [1] -http://ofbiz.135035.n4.nabble.com/Default-constructors-in-JAVA-classes-tp4749257p4749258.html[2] -https://github.com/danwatford/ofbiz-framework/tree/OFBIZ-11900-WIP[3] - https://plugins.gradle.org/plugin/io.freefair.lombok [4] - https://projectlombok.org/setup/gradle [5] - https://plugins.jetbrains.com/plugin/6317-lombok/ [6] - https://projectlombok.org/setup/overview-- Daniel Watford
