Alon Bar-Lev has posted comments on this change.

Change subject: engine: Add infrastructure code for removal of parameter classes
......................................................................


Patch Set 12:

(1 comment)

I like this! less code more happy, and I can even pretend I understand it.

I now start to review the patch set as a complete solution.

Will do this later today.

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VdcParameter.java
Line 86:             case 0:
Line 87:                 synchronized (parametersMap) {
Line 88:                     Guid guid = new Guid(map.get(GUID_KEY));
Line 89:                     if (!parametersMap.containsKey(guid)) {
Line 90:                         throw new 
InvalidParameterKeyException("VdcParameter with guid not found "+ 
guid.toString());
hmmm... won't it better to have:

 VdcParameter p = parametersMap.get(guid);
 if (p == null) {
     throw ...
 }
 return p;

this way you do not search the map twice.
Line 91:                     }
Line 92:                     return parametersMap.get(guid);
Line 93:                 }
Line 94:             default:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6a0970e492c0eff561888a46b02e47645ff68fc3
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: mooli tayer <mta...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to