Sergey Gotliv has posted comments on this change.

Change subject: core: Fixing some issues at SizeConverter
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

Yair, first welcome back.

Personally I didn't like the previous patch that created coverity issues in the 
first place. 

I think we need to remove some code from this class and hope that coverity 
issues will disappear with the code.

See my comments inside.

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/SizeConverter.java
Line 53:         long toType = toUnit.getUnitWeight();
Line 54:         return (size) * ((Math.pow(CONVERT_FACTOR, fromType)) / 
(Math.pow(CONVERT_FACTOR, toType)));
Line 55:     }
Line 56: 
Line 57:     public static Pair<SizeUnit, Double> autoConvert(Long size, 
SizeUnit inUnit) {
What is the problem here?
Why it should be Long, in convert function we using long and everything is 
perfect?
Line 58:         for (Pair<Long, SizeUnit> currentUnitPair : 
SizeUnit.weightToUnit) {
Line 59:             if (size.longValue() / Math.pow(CONVERT_FACTOR, 
currentUnitPair.getFirst() - inUnit.getUnitWeight()) >= 1) {
Line 60:                 return new Pair<SizeConverter.SizeUnit, 
Double>(currentUnitPair.getSecond(),
Line 61:                         Double.valueOf(SizeConverter.convert(size, 
inUnit, currentUnitPair.getSecond()).doubleValue()));


Line 61:                         Double.valueOf(SizeConverter.convert(size, 
inUnit, currentUnitPair.getSecond()).doubleValue()));
Line 62:             }
Line 63:         }
Line 64:         return new Pair<SizeConverter.SizeUnit, 
Double>(SizeUnit.BYTES, size.doubleValue());
Line 65:     }
Just casting to double will do the same job.
Line 66: 
Line 67:     private static class CompareUnits implements Comparator<Pair<Long, 
SizeUnit>>, Serializable {
Line 68: 
Line 69:         /**


Line 64:         return new Pair<SizeConverter.SizeUnit, 
Double>(SizeUnit.BYTES, size.doubleValue());
Line 65:     }
Line 66: 
Line 67:     private static class CompareUnits implements Comparator<Pair<Long, 
SizeUnit>>, Serializable {
Line 68: 
Please, replace this comparator class with the inline implementation. Its not 
needed and not used outside of this class.
Line 69:         /**
Line 70:          *
Line 71:          */
Line 72:         private static final long serialVersionUID = 
-659222322712056953L;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8115b1ccc18d199a9a31416e44923d9d05758c6
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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