rafaelweingartner commented on a change in pull request #2739: Async jobs add
endtime
URL: https://github.com/apache/cloudstack/pull/2739#discussion_r202324106
##########
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:
Sorry if I sound blunt, but yes I know this "excuse". My question is, what
are the cases that the use of these final keywords is helping us to avoid?
Is it the case that we have these monstrous methods (with 100+, or 500+
lines)? And then you are trying to help programmers to identify when he/she is
changing a variable that was previously assigned somewhere else in the middle
of a big chunk of code.
If that is the only reason for us to use this keyword, I do prefer other
options such as using shorter and concise methods, which can be unit tested.
Then, we can catch problems such as wrong assignments. Otherwise, we are just
adding code that is not needed.
----------------------------------------------------------------
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
