“For instance, when we have to design/create a library and we want to make sure an object is immutable.”
According to your argument, though, you don’t need final here either: just make sure to never provide setters or change those values (and document). > On Jan 20, 2018, at 9:37 AM, Tutkowski, Mike <mike.tutkow...@netapp.com> > wrote: > > 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
