Moti Asayag has posted comments on this change.
Change subject: core: enable null values for monitor, CD , Floppy is_plugged
property
......................................................................
Patch Set 1: (4 inline comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDevice.java
Line 29: private Guid deviceId;
if you intend to modify this class, the deviceId and vmId should be deleted
from this class as not being used and replaced by VmDeviceId.
Line 69: private Boolean isPlugged = false;
If the value of the isPlugged might be null, there is sort of a risk for NPE,
since any place calling the getIsPlugged() with null value will try to autobox
it into a primitive and fail for NPE.
The safe way is using Boolean.valueOf (getIsPlugged()) from places where this
getter is used in conditions. But then the third state of 'null' looses its
importance.
Line 169: this.specParams =
VmDeviceCommonUtils.appendDeviceIdToSpecParams(this.getId().getDeviceId(),
this.specParams);
"this." is redundant here .
Line 185: public Boolean getIsPlugged() {
not for this patch, but the convention says that getter of a boolean should
named without get prefix, e.g. isPlugged(), isManaged()...
--
To view, visit http://gerrit.ovirt.org/2408
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I826868106b7f27581838e96a985dbdeda4fa6aa2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
