Alissa Bonas has posted comments on this change.

Change subject: core: ImportVm cleanup: extract VM.hasImages()
......................................................................


Patch Set 1: (1 inline comment)

pls see  comment in code...

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java
Line 1015:         return mVmStatic.getImages();
Line 1016:     }
Line 1017: 
Line 1018:     public boolean hasImages() {
Line 1019:         return getImages().isEmpty();
Why is there a need in this additional method? Why not use directly getImages() 
and check them where the logic is needed if they are returned empty (something 
like  getImages().isEmpty() )?
and while I'm already here I'd like to ask, though it's not part of the change 
- is there a particular reason for getImages method to return an ArrayList 
rather than the List interface?
Line 1020:     }
Line 1021: 
Line 1022:     public void setImages(ArrayList<DiskImage> value) {
Line 1023:         mVmStatic.setImages(value);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7486e68268700bd0c55204dce6fb3f711425f931
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to