sureshanaparti commented on a change in pull request #5085:
URL: https://github.com/apache/cloudstack/pull/5085#discussion_r647998852



##########
File path: 
server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java
##########
@@ -45,6 +45,13 @@
 
     private SearchBuilder<ServiceOfferingJoinVO> sofIdSearch;
 
+    /**
+     * Constant used to convert GB into Bytes (or the other way around).
+     * GB   *  MB  *  KB  = Bytes //
+     * 1024 * 1024 * 1024 = 1073741824
+     */
+    private static final long GB_TO_BYTES = 1073741824;

Review comment:
       multiple constants are defined, in various classes, for the value "1024 
* 1024 * 1024", and it is hardcoded in some cases. may be (later), a constant 
can be defined in common package( eg. cloud utils), and use it across. (Such 
common util functions has to be documented, so that the developers can use 
these)
   
   Also, some get disk size calls return size in Bytes, and others return in 
GB. I think, it is better to explicitly mention (in variables / methods) it 
indicating Bytes or GB for non-default unit (i.e. If Bytes is the default unit, 
then methods which return in 'GB' should have it mentioned). If there is no 
such default unit, then variables / methods should indicate the size units 
explicitly. This can be a huge refactoring work (so can be skipped for now) and 
at least, have to make sure new code indicates proper size unit.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to