geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Owen Nichols <onich...@pivotal.io>
Subject Re: Unnecessary uses of final on local variables
Date Tue, 18 Jun 2019 22:22:50 GMT
I recommend:
* Use final if you want to
* Don’t -1 someone’s PR because they did or didn’t use final.


Personally, there is one case where I like to use final on a local variable: forcing the compiler
to tell me if I’ve covered all code paths.  Here’s a very simple example:

final int x;
if (cond) {
  x = 1;
}
else if(othercond) {
 x = 2;
}

//this will fail to compile because there is still one codepath where x never gets initialized.

Without final, you might be temped to write it more compactly as:

int x = 2;
if (cond) {
  x = 1;
}

//in this case, instead of a compiler error, you get a very subtle bug.  Let the compiler
work for you whenever possible!

This strategy is also handy if you’re initializing a variable with a switch statements,
because the final declaration will protect you from missing break statements.

> On Jun 18, 2019, at 3:02 PM, Murtuza Boxwala <mboxwala@pivotal.io> wrote:
> 
> What does "rough consensus"[1] look like on this thread? How do we make a decision and
close it out?
> 
> Kirk suggested an idea, there’s been a couple days of feedback, so we can:
> 
> 1) reject the proposal and commit to using final ‘everywhere'
> 2) accept the proposal and use final very sparingly
> 3) continue the discussion over e-mail
> 4) take other steps to come to a consensus like
> 	have a meeting with a few folks who are passionate about the topic
>        Try both approaches.  Pick one module and use final everywhere, and pick another
and use it sparingly and see which approach we like
> 
> 
> Personally, I don’t see any “fundamental flaws"[1] with using final or removing it.
> 
> 
> Also, I might just be rushing the natural flow of conversation, so in that case, sorry
for being impatient.
> 
> 1-https://doist.com/blog/decision-making-flat-organization/ <https://doist.com/blog/decision-making-flat-organization/>
> 
>> On Jun 18, 2019, at 2:48 PM, Jinmei Liao <jiliao@pivotal.io> wrote:
>> 
>> I agree with Murtuza, most finals on local variables and method parameters
>> are just noise to me. I only use "final" on these two situations:
>> 1. to declare public static constants of immutable types (e.g. String,
>> Integer)
>> 2. to prevent children from overriding a method.
>> 
>> But thought I can't offer an example, I don't want to put out a blank
>> statement saying that "final" on local variables are entirely unnecessary.
>> There must be a case that it could be useful, so even if we come to a
>> consensus that local variables should not have final on it, I don't think
>> using a static analysis tool to get rid of all of them is a good idea.
>> 
>> On Tue, Jun 18, 2019 at 11:14 AM Anthony Baker <abaker@pivotal.io> wrote:
>> 
>>> I’ll offer this alternative:  perhaps shorter method bodies obviate the
>>> need for explicit final vars.
>>> 
>>> Anthony
>>> 
>>> 
>>>> On Jun 18, 2019, at 10:30 AM, Ernest Burghardt <eburghardt@pivotal.io>
>>> wrote:
>>>> 
>>>> +1 to auto-enforcement (if possible) post-consensus
>>>> 
>>>> On Tue, Jun 18, 2019 at 8:33 AM Murtuza Boxwala <mboxwala@pivotal.io>
>>> wrote:
>>>> 
>>>>> final in Java does not guarantee immutability.  It would be AWESOME if
>>> it
>>>>> did but all it guarantees is that the variable cannot be reassigned.
In
>>>>> most cases the variable points to an object’s location (memory
>>> address), so
>>>>> you can still call methods on it, e.g.
>>>>> 
>>>>> final var = new Foo();
>>>>> var.mutateState();
>>>>> 
>>>>> final variables like these are in no way thread safe. To make objects
>>>>> immutable, the objects themselves need to follow a pattern that
>>> guarantees
>>>>> that.  Something like the ValueObject <
>>>>> https://martinfowler.com/bliki/ValueObject.html> pattern.
>>>>> 
>>>>> Mutability may well be the enemy, but I don’t think this is the
>>> construct
>>>>> that gets us much/if any closer.
>>>>> 
>>>>> In local variables and parameters final feels like noise to me, and in
>>>>> fact may make things more difficult to reason about, if we start
>>> assuming
>>>>> variables with final are thread safe.
>>>>> 
>>>>> But I may be missing something.  I am more curious to see how we come
to
>>>>> consensus on something like this, because the worst outcome from all
>>> this
>>>>> will be to have some folks actively adding final and some actively
>>> removing
>>>>> it, which will add noise to PRs and to the code.  And once we reach
>>>>> consensus, how do we enforce somethings like this? ./gradlew spA?
>>>>> 
>>>>>> On Jun 17, 2019, at 8:55 PM, Jacob Barrett <jbarrett@pivotal.io>
>>> wrote:
>>>>>> 
>>>>>> I too am in camp final too. You could say `final boolean useFinal
=
>>>>> true`. For all the same reasons Bill stated below.
>>>>>> 
>>>>>>> On Jun 17, 2019, at 5:33 PM, Bill Burcham <bburcham@pivotal.io>
>>> wrote:
>>>>>>> 
>>>>>>> The final keyword is not redundant—quite the opposite—it's
extremely
>>>>> valuable.
>>>>>>> 
>>>>>>> Local variables are not, in general, final, unless you declare
them as
>>>>> such. That being the case, it is not redundant to declare local
>>> variables
>>>>> "final".
>>>>>>> 
>>>>>>> What the compiler will do for you, is _if_ it can ensure that
a local
>>>>> variable (or method parameter) is never modified (after initialization)
>>>>> then that variable is treated as "effectively final". Variables that
are
>>>>> explicitly declared final, or are determined to be "effectively final"
>>> may
>>>>> be referenced in lambdas. That's a nice thing.
>>>>>>> 
>>>>>>> I would like to offer a counter-recommendation: final should
be the
>>>>> default everywhere for fields, for method parameters (on classes, not
on
>>>>> interfaces), and for local variables.
>>>>>>> 
>>>>>>> Many benefits would accrue to us, should we adopt this default:
>>>>>>> 
>>>>>>> 1. final fields must be initialized in a constructor and never
mutated
>>>>> again. This makes reasoning about those fields easier.
>>>>>>> 2. classes that have all their fields final are immutable and
hence
>>>>> easier to reason about: they can be passed between threads, for
>>> instance,
>>>>> with no need to protect from races
>>>>>>> 3. final method parameters can never be mutated, making them
easier to
>>>>> reason about
>>>>>>> 4. final local variables can never be mutated, making them easier
to
>>>>> reason about
>>>>>>> 
>>>>>>> When final is the rule, non-final is the exception. Another way
of
>>>>> saying that is that when final is the rule, mutability is the exception.
>>>>> That is as it should be. Mutability is the enemy.
>>>>>>> 
>>>>>>> I have turned on a couple IntelliJ settings that make this the
default
>>>>> for me. I encourage you to do the same:
>>>>>>> 
>>>>>>> First there are these two "Code style issues" in the Java inspections:
>>>>>>> 
>>>>>>> "Field may be 'final'"
>>>>>>> "Local variable or parameter can be final"
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> Then there is this setting will cause newly-defined variables
created
>>>>> via the "Extract variable" refactoring:
>>>>>>> 
>>>>>>> If you select that check box (after selecting those inspections
>>>>> settings above), it'll declare the newly-introduced variable "final"
and
>>>>> it'll remember the setting the next time you invoke "Extract variable"
>>>>> refactoring
>>>>>>> 
>>>>>>> 
>>>>> 
>>>>> 
>>> 
>>> 
>> 
>> -- 
>> Cheers
>> 
>> Jinmei
> 


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message