Arik Hadas has posted comments on this change.
Change subject: core: cleanup AddVmPoolWithVmsCommand#getJobMessageProperties
......................................................................
Patch Set 1: (1 inline comment)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmPoolWithVmsCommand.java
Line 83: public Map<String, String> getJobMessageProperties() {
Line 84: if (jobProperties == null) {
Line 85: jobProperties = new HashMap<String, String>();
Line 86: VmPool vmPool = getParameters().getVmPool();
Line 87: String vmPoolName = vmPool != null ?
vmPool.getVmPoolName() : StringUtils.EMPTY;
sorry, it's going to be a little long..
if you're looking for a convention I would propose something different: write
your if statements in such a way that the main scenario comes first and the
exception afterwards, because that way the code is more readable. of course you
need to adjust the if's condition accordingly.
I think that it's more important than writing positive or negative conditions
for simple conditions, because when you read the code you want to get the major
flow first and then think about the exceptions - in 99% of the cases where you
question whether something is different than null, you don't expect the object
to be null, if the object is null then you're taking default values or default
behaviour (exceptions).
in this specific case you want to get the pool-name, and only if it's null (it
shouldn't be) you're taking empty string.
anyway, IMO it's a very minor issue - I don't mind to change it in the next
patch
Line 88:
jobProperties.put(VdcObjectType.VmPool.name().toLowerCase(), vmPoolName);
Line 89: Guid vmTemplateId = getVmTemplateId();
Line 90: String templateName = getVmTemplateName();
Line 91: if (StringUtils.isEmpty(templateName)) {
--
To view, visit http://gerrit.ovirt.org/11983
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1513f901b7c5ef2b3d215ff119e0f69240c9411a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
