rafaelweingartner commented on a change in pull request #2739: Async jobs add 
endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202353309
 
 

 ##########
 File path: 
framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java
 ##########
 @@ -1049,8 +1054,8 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                             }
                         }
                     }
-                    List<SnapshotDetailsVO> snapshotList = 
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid), 
false);
-                    for (SnapshotDetailsVO snapshotDetailsVO : snapshotList) {
+                    final List<SnapshotDetailsVO> snapshotList = 
_snapshotDetailsDao.findDetails(AsyncJob.Constants.MS_ID, Long.toString(msid), 
false);
 
 Review comment:
   That is exactly my point. I have seen this indiscriminate use of the `final` 
keyword in some other places as if it would greatly improve our code base. This 
is also not the first time I call attention regarding the use of final.
   
   I really do not understand when I see people slapping `final` keywords in a 
method that has 100+ lines that can be extracted into smaller blocks. Then, we 
hear that it is a defensive programming style or some sort of best practice. I 
know, those expressions sound nice and shiny, but simply adding the `final` 
keyword makes no improvement what so ever. A poorly coded method will remain 
poorly coded with or without `final` keywords in its variables. 
   

----------------------------------------------------------------
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

Reply via email to