Gilad Chaplik has posted comments on this change.

Change subject: common: introduce qos package, and storage qos
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.ovirt.org/#/c/27093/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/BaseQos.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/BaseQos.java:

Line 17:  */
Line 18: public abstract class BaseQos extends IVdcQueryable implements 
BusinessEntity<Guid>, Serializable {
Line 19:     private static final long serialVersionUID = 1122772549710787678L;
Line 20:     private Guid id = Guid.Empty;
Line 21:     private QosType qoSType;
> about final, will get back to you.
will change to qosType
Line 22: 
Line 23:     @NotNull(message = "ACTION_TYPE_FAILED_NETWORK_QOS_INVALID_DC_ID")
Line 24:     private Guid storagePoolId;
Line 25: 


Line 32:     }
Line 33: 
Line 34:     public BaseQos(QosType qoSType) {
Line 35:         this();
Line 36:         this.qoSType = qoSType;
> otherwise you allow an object be initialized in a wrong way or spread the v
if(qosType == null) {
  throw new RunTimeException("qos type cannot be null");
}

okay?
Line 37:     }
Line 38:     /**
Line 39:      * @return object's type
Line 40:      */


Line 112:             return false;
Line 113:         if (getClass() != obj.getClass())
Line 114:             return false;
Line 115:         BaseQos other = (BaseQos) obj;
Line 116:         if (id == null) {
> IMHO that isn't an excuse for producing sub-optimal code.
Done
Line 117:             if (other.id != null)
Line 118:                 return false;
Line 119:         } else if (!id.equals(other.id))
Line 120:             return false;


http://gerrit.ovirt.org/#/c/27093/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/QosType.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/QosType.java:

Line 6: 
Line 7: public enum QosType implements Identifiable {
Line 8:     STORAGE(1);
Line 9: 
Line 10:     private int value;
> that's how enums is implemented in ovirt.
will re-consider.
Line 11:     private static final HashMap<Integer, QosType> valueToStatus = new 
HashMap<Integer, QosType>();
Line 12: 
Line 13:     static {
Line 14:         for (QosType status : values()) {


Line 9: 
Line 10:     private int value;
Line 11:     private static final HashMap<Integer, QosType> valueToStatus = new 
HashMap<Integer, QosType>();
Line 12: 
Line 13:     static {
> same.
will re-consider.
Line 14:         for (QosType status : values()) {
Line 15:             valueToStatus.put(status.getValue(), status);
Line 16:         }
Line 17:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1a9af59277b5055453159f002f19046c0051d8ff
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Kobi Ianko <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to