Mike Kolesnik has posted comments on this change.

Change subject: engine: Refactored code into NetworkValidator class
......................................................................


Patch Set 7: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/RemoveNetworkCommand.java
Line 48:             super(network);
Line 49:         }
Line 50: 
Line 51:         public ValidationResult notManagementNetwork() {
Line 52:             String managementNetwork = Config.<String> 
GetValue(ConfigValues.ManagementNetwork);
If this patch gets pushed:
http://gerrit.ovirt.org/#/c/11835

Then you could use NetworkHelper.managementNetwork method and save a couple or 
repeating lines :)
Line 53:             return 
managementNetwork.equalsIgnoreCase(network.getName())
Line 54:                     ? new 
ValidationResult(VdcBllMessages.NETWORK_CAN_NOT_REMOVE_DEFAULT_NETWORK)
Line 55:                     : ValidationResult.VALID;
Line 56:         }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkValidator.java
Line 45:      */
Line 46:     protected List<Network> getNetworks() {
Line 47:         if (networks == null) {
Line 48:             networks = 
getDbFacade().getNetworkDao().getAllForDataCenter(network.getDataCenterId());
Line 49:             if (networks == null) {
This code is not necessary, it's the DAO responsibility to return empty list if 
there are no networks.

That is the de-facto contract in the system.
Line 50:                 networks = Collections.<Network> emptyList();
Line 51:             }
Line 52:         }
Line 53:         return networks;


Line 116:                 : ValidationResult.VALID;
Line 117:     }
Line 118: 
Line 119:     /**
Line 120:      * @return An error iff the network doesn't exist.
Maybe also state in javadoc that the existence is checked by the network being 
set?
Since the actual error is that the network not exists, but we expect it to be 
set..
Line 121:      */
Line 122:     public ValidationResult networkIsSet() {
Line 123:         return network == null
Line 124:                 ? new 
ValidationResult(VdcBllMessages.NETWORK_NOT_EXISTS)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf40d81f0481c6b6dec141a71363888dc9e9a941
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Livnat Peer <lp...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to