Good point. We're probably stuck on vanilla JDK8 for years more anyway.
Since this discussion seems to be mainly about personal viewing preference, I
wonder if someone has written an IntellJ plugin that would simply render
effectively-final variables as if they were preceded by the word "final" (for
people that want to see finals everywhere), or a plugin to collapse unnecessary
finals (for people that don't want to see them).
On 4/21/21, 4:36 PM, "John Blum" <jb...@vmware.com> wrote:
I wouldn't recommend Lombok for production code, primarily because it
generates code. Generated code occasionally leads to unintended consequences or
incompatibilities with newer Java versions. Additionally, it requires as
3rd-party dependency to gain these capabilities rather than being part of the
Java language itself. I'd rather Java include appropriate features for these 2
concerns, i.e. val & var, much like Kotlin, to more succinctly express intent.
Even though Java now has var, it is different than Kotlin's var.
I only use Lombok for testing and demoing purposes.
-j
________________________________
From: Kirk Lund <kl...@apache.org>
Sent: Wednesday, April 21, 2021 12:59 PM
To: dev@geode.apache.org <dev@geode.apache.org>
Subject: Re: [VOTE] Requiring final keyword on every parameter and local
variable
I'm interested in
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprojectlombok.org%2F&data=04%7C01%7Conichols%40vmware.com%7C12ad0c3d5634430f31f808d9051e4d78%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637546449987523132%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=CDaPpdPe819GBlhtLa00DNXSi7HC%2Bm0weDUq2oHyGuo%3D&reserved=0
and I remember John Blum using
it on some projects. I would like to study it and do some perf testing on
it before supporting it for Geode though. In general, I don't like
generated code and it looks like at least some of the features involve
generated code -- that's the only potential downside for me.
-Kirk
On Mon, Apr 19, 2021 at 12:57 PM Owen Nichols <onich...@vmware.com> wrote:
> Any interest in adopting
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprojectlombok.org%2F&data=04%7C01%7Conichols%40vmware.com%7C12ad0c3d5634430f31f808d9051e4d78%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637546449987523132%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=CDaPpdPe819GBlhtLa00DNXSi7HC%2Bm0weDUq2oHyGuo%3D&reserved=0
in Geode? Lombok
> allow a more readable "val" vs "var" syntax instead of "final" vs "" which
> could make it easier to do the right thing without "increasing visual
> noise".
>
> On 4/19/21, 12:40 PM, "Dale Emery" <dem...@vmware.com> wrote:
>
> My test for whether enforce a guideline in a PR review is: Would I be
> willing to automate enforcing it in CI?
>
> I am -0 on this particular guideline.
>
> IntelliJ offers two competing inspections for Java coding style:
>
> * Local variable or parameter can be final
> * Unnecessary 'final' on local variable or parameter
>
> Both of these are (I think) disabled by default in IntelliJ. And
> neither satisfies my “I’d be willing to automate enforcing it” test.
>
> Dale
>
> From: Mark Hanson <hans...@vmware.com>
> Date: Monday, April 19, 2021 at 11:08 AM
> To: dev@geode.apache.org <dev@geode.apache.org>
> Subject: Re: [VOTE] Requiring final keyword on every parameter and
> local variable
> Hi Jake,
>
> I agree with everyone's point about final being a useful, I just don't
> find it useful enough to do anything manually across the code base at this
> point.
>
> I believe first in foremost in no code warnings. Intellij warns about
> variables that can be final, so I make them final as it finds them. It is
> very rare that I am writing new code at this point. I have spent the last
> year bug fixing other people's code. From my standpoint, original intent
> is largely moot. So, for me, the question is do I go through code that is
> not mine and mark it all as final (where appropriate)? Sure, in code that
I
> touch. In code the rest of the codebase, I think there are bigger fish to
> fry before we get to final.
>
> It seems like the larger portion of the consensus is to recommend that
> variables are marked final (where appropriate) as we find them or create
> them.
> That seems like the going forward approach consensus.
>
> I will be blunt though. This seems nitpicky *compared* to the number
> of code warnings there are and the fact that people are not actively
fixing
> all of the warnings.
> If we don't come to the same consensus about all warnings this seems
> like painting a rusted car, sure it makes it all look the same, but does
> very little for the underlying problems. Now to undermine my own argument
a
> little, I think that especially in release blockers, we want to touch as
> little code as possible, so I am flexible there.
>
> I would also like to agree with Udo about final really not being very
> good compared to Mutable and Immutable objects in other languages.
>
> Thanks,
> Mark
>
>
> On 4/15/21, 7:49 PM, "Udo Kohlmeyer" <u...@vmware.com> wrote:
>
> @jake, you are correct, I did miss the LOCAL variable out of the
> subject. :face_palm:
>
> Yes, adding "final" to LOCAL variables will be HUGELY beneficial
> in this regard. Have we seen any performance improvement after having
> refactored this method? Have you been able to measure any improvements?
>
> --Udo
>
> On 4/15/21, 1:19 PM, "Jacob Barrett" <jabarr...@vmware.com> wrote:
>
>
>
> > On Apr 14, 2021, at 7:46 PM, Udo Kohlmeyer <u...@vmware.com>
> wrote:
> > @Jake the idea of smaller methods is great and we should
> ALWAYS strive for that. But that argument is completely irrelevant in this
> discussion. As making method arguments final does not naturally guide a
> developer to creating smaller methods. Nor does a smaller method mean it
> can/will be jitted. Too many factors (to discuss here) are part of that
> decision, also it is not relevant in this discussion. But more on that
> topic read THIS.
>
> The original subject is in regards to parameters and local
> variables.
>
> Irrelevant is certainly an opinion you are welcome to have but
> let me challenge you. Goto DistributedCacheOperation._distribute().
>
> First challenge, look at around line 333:
> boolean reliableOp = isOperationReliable() &&
> region.requiresReliabilityCheck();
>
> Without scrolling do you see that variable used? Nope, because
> it is first used on line 439, ~100 lines away. Does it mutate between
> there, well I can search for all uses and find out or I could be nice to
> the next person and intend for it to never mutate by adding final. Intent
> communicated!
>
> Second challenge, mark all the local variables in the method
> as final. Now make it compile without introducing more mutable variables.
> At the end of this journey you will have about a dozen unit testable
> methods and a _distribute method that goes from ~370 lines to ~90 with no
> mutable local variables.
>
> I argue it is relevant as good guardrail for writing good
> code. While we should ALWAYS strive for it we don’t. Every little nudge
> helps.
>
>
> -Jake
>
>
>
>
