Liron Aravot has posted comments on this change.
Change subject: core: Extract code to methods in ImagesHandler
......................................................................
Patch Set 1: (3 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
Line 437: break;
Line 438: }
Line 439: return result;
Line 440: }
Line 441:
one of this methods gets an entity and returns if in the needed status or not,
one gets an id - i'd suggest that they both recieve an entity as parameter so
we won't perform dao call in chance we already got the entity.
Line 442:
Line 443: public static boolean isVmDown(VM vm) {
Line 444: return vm.getStatus() == VMStatus.Down;
Line 445: }
Line 438: }
Line 439: return result;
Line 440: }
Line 441:
Line 442:
I'd put this in VmHandler, i'd also make the method more generic(get status as
a parameter) if we already add a method for this
Line 443: public static boolean isVmDown(VM vm) {
Line 444: return vm.getStatus() == VMStatus.Down;
Line 445: }
Line 446:
Line 444: return vm.getStatus() == VMStatus.Down;
Line 445: }
Line 446:
Line 447:
Line 448: public static boolean isStoragePoolValid(Guid storagePoolId) {
1. i'd suggest just return true/false and remove this boolean decleration - no
need for it..iin one place we return false and in one place we return true.
2. if added, it should be private or in a different class..i don't think that
people should call images handler to check storage pool status.
3 . i think that it's better to get storage pool as parameter instead of
loading it within this method..right now the method is
"loadStoragePoolAndCheckValid", i prefer to have it to doing one operation..
Line 449: boolean isValid = true;
Line 450: storage_pool pool =
DbFacade.getInstance().getStoragePoolDao().get(storagePoolId);
Line 451: if (pool == null || pool.getstatus() != StoragePoolStatus.Up)
{
Line 452: isValid = false;
--
To view, visit http://gerrit.ovirt.org/10148
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I363f7bf24ca9b5d408a61ba9159b6e5f2a5c46e0
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
