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