Moti Asayag has posted comments on this change.

Change subject: gluster : In the task tab, size of rebalanced files is shown 
with units
......................................................................


Patch Set 16:

(3 comments)

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/SizeConverter.java
Line 39:         long fromType = fromUnit.getUnitWeight();
Line 40:         long toType = toUnit.getUnitWeight();
Line 41:         return (size) * ((Math.pow(CONVERT_FACTOR, fromType)) / 
(Math.pow(CONVERT_FACTOR, toType)));
Line 42:     }
Line 43: 
please use the formatter for the code.
Line 44:     public static String autoConvert(long size , SizeUnit inUnit) {
Line 45:         /*
Line 46:         NumberFormat formatSize = NumberFormat.getInstance();
Line 47:         formatSize.setMaximumFractionDigits(2);


Line 41:         return (size) * ((Math.pow(CONVERT_FACTOR, fromType)) / 
(Math.pow(CONVERT_FACTOR, toType)));
Line 42:     }
Line 43: 
Line 44:     public static String autoConvert(long size , SizeUnit inUnit) {
Line 45:         /*
please remove the commented lines.
Line 46:         NumberFormat formatSize = NumberFormat.getInstance();
Line 47:         formatSize.setMaximumFractionDigits(2);
Line 48:         formatSize.setMinimumFractionDigits(2);
Line 49:         */


Line 46:         NumberFormat formatSize = NumberFormat.getInstance();
Line 47:         formatSize.setMaximumFractionDigits(2);
Line 48:         formatSize.setMinimumFractionDigits(2);
Line 49:         */
Line 50:         for(long i = (SizeUnit.values())[SizeUnit.values().length - 
1].getUnitWeight(); i >= inUnit.getUnitWeight(); i-- ) {
This code relies on the implementation of the values() method of the enum type.

It also requires the user which adds additional entity to the enum to maintain 
its order.

wouldn't you rather having an ordered list (as this code assumes) in SizeUnit 
so you can iterate over them in a safer manner ?
Line 51:             if(size/Math.pow(CONVERT_FACTOR, i-1) >= 1) {
Line 52:                 return ((String.valueOf(convert(size, inUnit, 
SizeUnit.getUnit(i)).floatValue())).concat(" 
")).concat(SizeUnit.getUnit(i).toString());
Line 53:             }
Line 54:         }


-- 
To view, visit http://gerrit.ovirt.org/20511
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea6c90a098bfddfb616bc2b8ce58c9d0bb567f66
Gerrit-PatchSet: 16
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: anmolbabu <anb...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com>
Gerrit-Reviewer: anmolbabu <anb...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to