Hi Aljoscha,
I totally agree with you on field topic. Of course it makes significant
difference whether
or not a field is final and IDE/compiler can help on checking.
Here are several thoughts about final modifier on parameters and why I
propose this one:
1. parameters should be always final
As described above, there is no reason a parameter to be non-final. So
different with field,
a field can be final or non-final based on whether or not it is immutable.
Thus with such a
code style guide in our community, we can work towards a codebase every
parameter is
effectively final.
2. parameter final cannot be inherited
Unfortunately Java doesn't keep final modifier of method parameter when
inheritance. So
even you mark a parameter as final in an interface or super class, you have
to re-mark it
as final in its implementation or subclass. From another perspective, final
modifier of
parameter is a local attribute of parameter so that we can narrow possible
effect during
review.
3. IDE can lint difference between effective final and mutable parameter
It is true that IDE such as Intellij IDEA can lint difference between
effective final and
mutable parameter(with underline). So that with this code style what we
lose is that
we cannot get a compile time error if someone modifies parameter in the
method body.
But as mentioned in 1, by no means we allow a parameter to be modified. If
we agree
on this statement, then we hopefully converge in a codebase that no
parameter is
modified.
For what we gain, I'd like to recur our existing code style of @Nonnull to
be default.
Actually it does help for compiler to report compile time warning if we
possibly pass a
nullable value to an non-null field. We make @Nonnull as default to "reduce
code noise"
so I think we can reuse the statement here also.
Best,
tison.
Aljoscha Krettek <aljos...@apache.org> 于2019年10月4日周五 下午5:58写道：
> I actually think that doing this the other way round would be correct.
> Removing final everywhere and relying on humans to assume that everything
> is final doesn’t seem maintainable to me. The benefit of having final on
> parameters/fields is that the compiler/IDE actually checks that you don’t
> modify it.
>
> In general, I think that as much as possible should be declared final,
> including fields and parameters.
>
> Best,
> Aljoscha
>
> > On 2. Oct 2019, at 13:31, Piotr Nowojski <pi...@ververica.com> wrote:
> >
> > +1 from my side.
> >
> >> On 2 Oct 2019, at 13:07, Zili Chen <wander4...@gmail.com> wrote:
> >>
> >> Yes exactly.
> >>
> >>
> >> Piotr Nowojski <pi...@ververica.com> 于2019年10月2日周三 下午7:03写道：
> >>
> >>> Hi Tison,
> >>>
> >>> To clarify your proposal. You are proposing to actually drop the
> >>> `final` keyword from the parameters and we should implicilty assume
> that
> >>> it’s always there (in other words, we shouldn’t be modifying the
> >>> parameters). Did I understand this correctly?
> >>>
> >>> Piotrek
> >>>
> >>>> On 1 Oct 2019, at 21:44, Zili Chen <wander4...@gmail.com> wrote:
> >>>>
> >>>> Hi devs,
> >>>>
> >>>> Coming from this discussion[1] I'd like to propose that in Flink
> codebase
> >>>> we suggest a code style
> >>>> that parameters of method are always final. Almost everywhere
> parameters
> >>> of
> >>>> method are final
> >>>> already and if we have such consensus we can prevent redundant final
> >>>> modifier in method
> >>>> declaration so that we survive from those noise.
> >>>>
> >>>> Here are some cases that might require to modify a parameter.
> >>>>
> >>>> 1. to set default; especially if (param == null) { param = ... }
> >>>> 2. to refine parameter; it is in pattern if ( ... ) { param =
> >>>> refine(param); }
> >>>>
> >>>> Either of the cases we can move the refine/set default logics to the
> >>> caller
> >>>> or select another
> >>>> name for the refined value case by case.
> >>>>
> >>>> Looking forward to your feedbacks :-)
> >>>>
> >>>> Best,
> >>>> tison.
> >>>>
> >>>> [1] https://github.com/apache/flink/pull/9788#discussion_r329314681
> >>>
> >>>
> >
>
>
