Argy! Sorry typo (corrected with surrounding ***)...
I vote -1 for (generally) using final within constructor/method parameters and
local variables.
While final (e.g. on instance variables) has guaranteed benefits during
construction (e.g. initialization safety of an object used in a muti-Threaded
context) as Kirk rightly pointed out, its non-apparent, intended use can be
ill-conceived and ill-received outside this context.
As Jason also mentioned, which cannot be overemphasized enough, final (by
itself) does not guarantee immutability, which is often the intention of using
final in the first place.
While using final ensures immutability of primitive types (e.g. int, & their
corresponding wrapper types, i.e. Integer, only because the Integer class is
immutable to begin with) it does not ensure immutability of reference types
(***mutable objects***), arrays or Collection types. You can still change
primitive arrays or arrays/Collections containing reference types, even when
the array/Collection reference is final, regardless of whether an
array/Collection is passed to a constructor or method. Additionally, arrays and
Collections may contain mutable objects.
While there is some utility in using final, there are often much better
techniques that can be applied, such as, but not limited to:
1) Strict validation and read-only use of arguments (effectively immutable).
2) Making defensive copies of arguments passed in where appropriate.
3) Limiting the scope of the (defined) variable. E.g. Using (private) utility
methods to limit the scope and use (processing) of the variable passed into a
constructor or method as an argument.
Keep in mind that Java is Pass by Value, not Pass by Reference, and in some
cases, you can use this fact to your advantage.
So, because final is not a complete solution, this is why I vote -1.
NOTE: This does not mean I don't support using final in some cases (even for
constructor or method parameters and even local variables), just that it should
be used judiciously, like everything else, to explicitly express intent. And,
most importantly, function should match intent.
-j
________________________________
From: John Blum <jb...@vmware.com>
Sent: Wednesday, April 21, 2021 6:02 PM
To: dev@geode.apache.org <dev@geode.apache.org>
Subject: Re: [VOTE] Requiring final keyword on every parameter and local
variable
I vote -1 for (generally) using final within constructor/method parameters and
local variables.
While final (e.g. on instance variables) has guaranteed benefits during
construction (e.g. initialization safety of an object used in a muti-Threaded
context) as Kirk rightly pointed out, its non-apparent, intended use can be
ill-conceived and ill-received outside this context.
As Jason also mentioned, which cannot be overemphasized enough, final (by
itself) does not guarantee immutability, which is often the intention of using
final in the first place.
While using final ensures immutability of primitive types (e.g. int, & their
corresponding wrapper types, i.e. Integer, only because the Integer class is
immutable to begin with) it does not ensure immutability of reference types
(immutable objects), arrays or Collection types. You can still change
primitive arrays or arrays/Collections containing reference types, even when
the array/Collection reference is final, regardless of whether an
array/Collection is passed to a constructor or method. Additionally, arrays and
Collections may contain mutable objects.
While there is some utility in using final, there are often much better
techniques that can be applied, such as, but not limited to:
1) Strict validation and read-only use of arguments (effectively immutable).
2) Making defensive copies of arguments passed in where appropriate.
3) Limiting the scope of the (defined) variable. E.g. Using (private) utility
methods to limit the scope and use (processing) of the variable passed into a
constructor or method as an argument.
Keep in mind that Java is Pass by Value, not Pass by Reference, and in some
cases, you can use this fact to your advantage.
So, because final is not a complete solution, this is why I vote -1.
NOTE: This does not mean I don't support using final in some cases (even for
constructor or method parameters and even local variables), just that it should
be used judiciously, like everything else, to explicitly express intent. And,
most importantly, function should match intent.
-j
________________________________
From: Owen Nichols <onich...@vmware.com>
Sent: Wednesday, April 21, 2021 5:16 PM
To: dev@geode.apache.org <dev@geode.apache.org>
Subject: Re: [VOTE] Requiring final keyword on every parameter and local
variable
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%7Cjblum%40vmware.com%7C289192b0f0e1448ab80008d9052a5047%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637546501568451216%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=C5enkn9VDMUBjDtyJN3PTTDZyy0MGeLzWvz1dv6srGs%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%7Cjblum%40vmware.com%7C289192b0f0e1448ab80008d9052a5047%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637546501568461213%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=TCcWHcQaOOcpZ4n7s%2Bgrl4RzHmR8tc40%2B1Mp7Yi6%2F%2Bw%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
>
>
>
>
