Personally, I’m OK with it either way here. If @wido reads what you wrote and asks me to change it back to the way I wrote it initially, I’m happy to do so.
I believe he sees the explicitly declared constant here not only as a protection against ourselves, but to prevent a future programmer from easily making a mistake. At least now, if this future programmer changes the value, the compile will fail and he/she will be forced to think through the repercussions of doing so. > On Jan 20, 2018, at 9:30 AM, Rafael Weingärtner <rafaelweingart...@gmail.com> > wrote: > > Never is a strong word. In any case, let it be if you believe it is going > to provide benefits… > > I believe `final` modifier should be used in certain situation when it is > truly required. For instance, when we have to design/create a library and > we want to make sure an object is immutable. Then, we configure all of its > variables as final. Now, declaring a variable final in the middle of a > method to protect against ourselves…. It does not seem to bring much > benefit against future mistaken/accidental changes. It is always possible > for the hypothetical future programmer to remove the final and change the > variable. Perhaps a better documentation for the method explaining this > situations could bring more benefits. A single variable declared as final > does not give a hint for people on why it was made final. > > On Sat, Jan 20, 2018 at 2:08 PM, Tutkowski, Mike <mike.tutkow...@netapp.com> > wrote: > >> Perhaps your argument against the final keyword is that it should never be >> used? If so, you and @wido can debate that and let me know which way you’d >> like this bit of code to end up. >> >>> On Jan 20, 2018, at 9:05 AM, Tutkowski, Mike <mike.tutkow...@netapp.com> >> wrote: >>> >>> It’s pretty much the reason why the final variable exists in Java: to >> make a variable a constant so no one accidentally changes it. @wido just >> wanted the code to protect against the variable being changed...that’s all. >>> >>>> On Jan 20, 2018, at 8:25 AM, Rafael Weingärtner < >> rafaelweingart...@gmail.com> wrote: >>>> >>>> Changed by a future programmer that may develop some changes in your >> code? >>>> Because, if you do not code any other assignment to that variable, it >> will >>>> never happen. >>>> >>>> On Sat, Jan 20, 2018 at 1:16 PM, Tutkowski, Mike < >> mike.tutkow...@netapp.com> >>>> wrote: >>>> >>>>> @wido just wanted to make sure the Boolean variable didn’t accidentally >>>>> get changed later since it’s critical it keep that value. >>>>> >>>>>> On Jan 20, 2018, at 8:14 AM, Tutkowski, Mike < >> mike.tutkow...@netapp.com> >>>>> wrote: >>>>>> >>>>>> I made it final per @wido’s comment (in #2415). How’s about you guys >>>>> hash it out and get back to me. :) >>>>>> >>>>>>> On Jan 20, 2018, at 8:06 AM, GitBox <g...@apache.org> wrote: >>>>>>> >>>>>>> rafaelweingartner commented on a change in pull request #2416: >>>>> CLOUDSTACK-10244: KVM online storage migration fails >>>>>>> URL: https://github.com/apache/cloudstack/pull/2416# >>>>> discussion_r162784630 >>>>>>> >>>>>>> >>>>>>> >>>>>>> ########## >>>>>>> File path: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/ >>>>> resource/wrapper/LibvirtMigrateCommandWrapper.java >>>>>>> ########## >>>>>>> @@ -132,8 +132,9 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to >>>>> v1.0.0. >>>>>>> vmsnapshots = libvirtComputingResource. >>>>> cleanVMSnapshotMetadata(dm); >>>>>>> >>>>>>> Map<String, MigrateCommand.MigrateDiskInfo> >>>>> mapMigrateStorage = command.getMigrateStorage(); >>>>>>> + final boolean migrateStorage = MapUtils.isNotEmpty( >>>>> mapMigrateStorage); >>>>>>> >>>>>>> Review comment: >>>>>>> This variable does not need to be final. It does not bring any >>>>> benefits here. >>>>>>> >>>>>>> You needed to assign the result of the first `MapUtils.isNotEmpty( >> mapMigrateStorage)` >>>>> because the method `replaceStorage` might change the status of >>>>> `mapMigrateStorage`, right? >>>>>>> >>>>>>> ---------------------------------------------------------------- >>>>>>> This is an automated message from the Apache Git Service. >>>>>>> To respond to the message, please log on GitHub and use the >>>>>>> URL above to go to the specific comment. >>>>>>> >>>>>>> For queries about this service, please contact Infrastructure at: >>>>>>> us...@infra.apache.org >>>>>>> >>>>>>> >>>>>>> With regards, >>>>>>> Apache Git Services >>>>> >>>> >>>> >>>> >>>> -- >>>> Rafael Weingärtner >> > > > > -- > Rafael Weingärtner
