Arik Hadas has uploaded a new change for review.

Change subject: core: organize AddVmCommand#canDoAction
......................................................................

core: organize AddVmCommand#canDoAction

Refactoring AddVmCommand#canDoAction method, such that it won't use the
local variable returnValue to store the result - instead, we return
false as soon as we detect a required condition that is false. those
changes make the code more readable and compact.

Change-Id: Ie4eda8868b50fd4a99462c0a649bba58e52f9bc0
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
1 file changed, 35 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/41/15841/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
index c70a1a8..373abf5 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
@@ -280,44 +280,45 @@
 
     @Override
     protected boolean canDoAction() {
-        boolean returnValue = true;
         if (getVmTemplate() == null) {
-            
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST);
-            return false;
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST);
         }
+
         if (getVmTemplate().isDisabled()) {
             return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_IS_DISABLED);
         }
-        returnValue = buildAndCheckDestStorageDomains();
-        if (returnValue) {
-            storageToDisksMap =
-                    
ImagesHandler.buildStorageToDiskMap(getImagesToCheckDestinationStorageDomains(),
-                            diskInfoDestinationMap);
-            returnValue = canDoAddVmCommand();
+
+        if (!buildAndCheckDestStorageDomains()) {
+            return false;
+        }
+
+        storageToDisksMap =
+                
ImagesHandler.buildStorageToDiskMap(getImagesToCheckDestinationStorageDomains(),
+                        diskInfoDestinationMap);
+        if (!canDoAddVmCommand()) {
+            return false;
         }
 
         String vmName = getParameters().getVm().getName();
         if (vmName == null || vmName.isEmpty()) {
-            returnValue = false;
-            
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NAME_MAY_NOT_BE_EMPTY);
-        } else {
-            // check that VM name is not too long
-            boolean vmNameValidLength = 
isVmNameValidLength(getParameters().getVm());
-            if (!vmNameValidLength) {
-                returnValue = false;
-                
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NAME_LENGTH_IS_TOO_LONG);
-            }
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_MAY_NOT_BE_EMPTY);
+        }
+
+        // check that VM name is not too long
+        if (!isVmNameValidLength(getParameters().getVm())) {
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_LENGTH_IS_TOO_LONG);
         }
 
         // check for Vm Payload
-        if (returnValue && getParameters().getVmPayload() != null) {
-            returnValue = checkPayload(getParameters().getVmPayload(),
-                    getParameters().getVmStaticData().getIsoPath());
-            if (returnValue) {
-                // we save the content in base64 string
-                
getParameters().getVmPayload().setContent(Base64.encodeBase64String(
-                        
getParameters().getVmPayload().getContent().getBytes()));
+        if (getParameters().getVmPayload() != null) {
+            if (!checkPayload(getParameters().getVmPayload(),
+                    getParameters().getVmStaticData().getIsoPath())) {
+                return false;
             }
+
+            // otherwise, we save the content in base64 string
+            
getParameters().getVmPayload().setContent(Base64.encodeBase64String(
+                    getParameters().getVmPayload().getContent().getBytes()));
         }
 
         // Check that the USB policy is legal
@@ -325,13 +326,13 @@
                 getParameters().getVm().getOs(),
                 getVdsGroup(),
                 getReturnValue().getCanDoActionMessages())) {
-            returnValue = false;
+            return false;
         }
 
         // check cpuPinning if the check haven't failed yet
-        if (returnValue) {
-            VM vmFromParams = getParameters().getVm();
-            returnValue = isCpuPinningValid(vmFromParams.getCpuPinning(), 
vmFromParams.getStaticData());
+        VM vmFromParams = getParameters().getVm();
+        if (!isCpuPinningValid(vmFromParams.getCpuPinning(), 
vmFromParams.getStaticData())) {
+            return false;
         }
 
         if (getParameters().getVm().isUseHostCpuFlags()
@@ -339,7 +340,11 @@
             return 
failCanDoAction(VdcBllMessages.VM_HOSTCPU_MUST_BE_PINNED_TO_HOST);
         }
 
-        return returnValue && checkCpuSockets();
+        if (!checkCpuSockets()){
+            return false;
+        }
+
+        return true;
     }
 
     protected boolean checkCpuSockets() {


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie4eda8868b50fd4a99462c0a649bba58e52f9bc0
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to