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