Re: A little bit of Lombok

2020-08-26 Thread Shi Jinghai
Just my 2 cents.

-1 for using Lombok plugin in OFBiz.

I had some not good experiences with this plugin in Apereo CAS 5.3.x.



发送自 Windows 10 版邮件应用

发件人: Daniel Watford
发送时间: 2020年7月28日 20:44
收件人: dev@ofbiz.apache.org
主题: A little bit of Lombok

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 (refactoring MacroFormRenderer)
I have used Lombok on a few small classes. The work-in-progress branch can
be found at [2].

Only a small amount of Lombok has been used so far, meaning it shouldn't be
too 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 resolve this I
increased 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 used by
the Ofbiz project.

You will likely need your IDE to apply annotation processing otherwise you
might see warning of missing methods. In IntelliJ I use what appears to be
the de facto Lombok plugin [5].
Guidance is available from the Lombok project for other IDEs [6].

Lombok @ToString annotations have been applied to RenderableFtlString and
RenderableFtlNoop. This causes Lombok to insert toString() methods into the
classes 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 that Lombok
inserts for us.

To see the sources generated by Lombok we can run DeLombok. At the command
line 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 delomboked sources
for 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



Re: A little bit of Lombok

2020-08-26 Thread Jacques Le Roux

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  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.

Re: A little bit of Lombok

2020-08-26 Thread Daniel Watford
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  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