Fix technical debt and reduce lines
Focused on iterating over entrySet along with more fixes for
dead stores, unnecessary exceptions. I'm also starting to work
on reducing the number of code lines.
* Iterate using entrySet
* Remove useless assignments
* Unnecessary exceptions
* Use StringBuilder instead of StringBuffer due to synchronization
* Potential null exceptions being thrown
* Returning a empty collection vs null
Issue-ID: POLICY-482
Change-Id: If6ac8e812237f37b2b10c534535df4090a5073dd
Signed-off-by: Pamela Dragosh <pdragosh@research.att.com>
diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyAdapter.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyAdapter.java
index 545143a..42010f1 100644
--- a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyAdapter.java
+++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyAdapter.java
@@ -41,33 +41,26 @@
private static final Logger LOGGER = FlexLogger.getLogger(PolicyAdapter.class);
public void configure(PolicyRestAdapter policyAdapter, PolicyEntity entity) {
- String policyNameValue;
- String configPolicyName = null ;
if(extendedOptions(policyAdapter, entity)){
return;
}
+ String policyNameValue = policyAdapter.getPolicyName().substring(0, policyAdapter.getPolicyName().indexOf('_'));
+ String configPolicyName = null ;
if(policyAdapter.getPolicyName().startsWith("Config_PM")){
- policyNameValue = policyAdapter.getPolicyName().substring(0, policyAdapter.getPolicyName().indexOf('_'));
configPolicyName = "ClosedLoop_PM";
}else if(policyAdapter.getPolicyName().startsWith("Config_Fault")){
- policyNameValue = policyAdapter.getPolicyName().substring(0, policyAdapter.getPolicyName().indexOf('_'));
configPolicyName = "ClosedLoop_Fault";
}else if(policyAdapter.getPolicyName().startsWith("Config_FW")){
- policyNameValue = policyAdapter.getPolicyName().substring(0, policyAdapter.getPolicyName().indexOf('_'));
configPolicyName = "Firewall Config";
}else if(policyAdapter.getPolicyName().startsWith("Config_BRMS_Raw")){
- policyNameValue = policyAdapter.getPolicyName().substring(0, policyAdapter.getPolicyName().indexOf('_'));
configPolicyName = "BRMS_Raw";
}else if(policyAdapter.getPolicyName().startsWith("Config_BRMS_Param")){
- policyNameValue = policyAdapter.getPolicyName().substring(0, policyAdapter.getPolicyName().indexOf('_'));
configPolicyName = "BRMS_Param";
}else if(policyAdapter.getPolicyName().startsWith("Config_MS")){
- policyNameValue = policyAdapter.getPolicyName().substring(0, policyAdapter.getPolicyName().indexOf('_'));
configPolicyName = "Micro Service";
}else if(policyAdapter.getPolicyName().startsWith("Action") || policyAdapter.getPolicyName().startsWith("Decision") ){
- policyNameValue = policyAdapter.getPolicyName().substring(0, policyAdapter.getPolicyName().indexOf('_'));
+ // No configPolicyName is applicable
}else{
- policyNameValue = policyAdapter.getPolicyName().substring(0, policyAdapter.getPolicyName().indexOf('_'));
configPolicyName = "Base";
}
if (policyNameValue != null) {
@@ -78,41 +71,32 @@
}
if("Action".equalsIgnoreCase(policyAdapter.getPolicyType())){
- ActionPolicyController actionController = new ActionPolicyController();
- actionController.prePopulateActionPolicyData(policyAdapter, entity);
+ new ActionPolicyController().prePopulateActionPolicyData(policyAdapter, entity);
}
if("Decision".equalsIgnoreCase(policyAdapter.getPolicyType())){
- DecisionPolicyController decisionController = new DecisionPolicyController();
- decisionController.prePopulateDecisionPolicyData(policyAdapter, entity);
+ new DecisionPolicyController().prePopulateDecisionPolicyData(policyAdapter, entity);
}
if("Config".equalsIgnoreCase(policyAdapter.getPolicyType())){
if("Base".equalsIgnoreCase(policyAdapter.getConfigPolicyType())){
- CreatePolicyController baseController = new CreatePolicyController();
- baseController.prePopulateBaseConfigPolicyData(policyAdapter, entity);
+ new CreatePolicyController().prePopulateBaseConfigPolicyData(policyAdapter, entity);
}
else if("BRMS_Raw".equalsIgnoreCase(policyAdapter.getConfigPolicyType())){
- CreateBRMSRawController brmsController = new CreateBRMSRawController();
- brmsController.prePopulateBRMSRawPolicyData(policyAdapter, entity);
+ new CreateBRMSRawController().prePopulateBRMSRawPolicyData(policyAdapter, entity);
}
else if("BRMS_Param".equalsIgnoreCase(policyAdapter.getConfigPolicyType())){
- CreateBRMSParamController paramController = new CreateBRMSParamController();
- paramController.prePopulateBRMSParamPolicyData(policyAdapter, entity);
+ new CreateBRMSParamController().prePopulateBRMSParamPolicyData(policyAdapter, entity);
}
else if("ClosedLoop_Fault".equalsIgnoreCase(policyAdapter.getConfigPolicyType())){
- CreateClosedLoopFaultController newFaultTemplate = new CreateClosedLoopFaultController();
- newFaultTemplate.prePopulateClosedLoopFaultPolicyData(policyAdapter, entity);
+ new CreateClosedLoopFaultController().prePopulateClosedLoopFaultPolicyData(policyAdapter, entity);
}
else if("ClosedLoop_PM".equalsIgnoreCase(policyAdapter.getConfigPolicyType())){
- CreateClosedLoopPMController pmController = new CreateClosedLoopPMController();
- pmController.prePopulateClosedLoopPMPolicyData(policyAdapter, entity);
+ new CreateClosedLoopPMController().prePopulateClosedLoopPMPolicyData(policyAdapter, entity);
}
else if("Micro Service".equalsIgnoreCase(policyAdapter.getConfigPolicyType())){
- CreateDcaeMicroServiceController msController = new CreateDcaeMicroServiceController();
- msController.prePopulateDCAEMSPolicyData(policyAdapter, entity);
+ new CreateDcaeMicroServiceController().prePopulateDCAEMSPolicyData(policyAdapter, entity);
}
else if("Firewall Config".equalsIgnoreCase(policyAdapter.getConfigPolicyType())){
- CreateFirewallController firewallController = new CreateFirewallController();
- firewallController.prePopulateFWPolicyData(policyAdapter, entity);
+ new CreateFirewallController().prePopulateFWPolicyData(policyAdapter, entity);
}
}
}
@@ -130,7 +114,5 @@
}
return null;
}
-
-
}
diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyRestController.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyRestController.java
index d930a6d..63d0cb2 100644
--- a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyRestController.java
+++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyRestController.java
@@ -141,14 +141,11 @@
if(policyData.getConfigPolicyType() != null){
if("ClosedLoop_Fault".equalsIgnoreCase(policyData.getConfigPolicyType())){
- CreateClosedLoopFaultController faultController = new CreateClosedLoopFaultController();
- policyData = faultController.setDataToPolicyRestAdapter(policyData, root);
+ policyData = new CreateClosedLoopFaultController().setDataToPolicyRestAdapter(policyData, root);
}else if("Firewall Config".equalsIgnoreCase(policyData.getConfigPolicyType())){
- CreateFirewallController fwController = new CreateFirewallController();
- policyData = fwController.setDataToPolicyRestAdapter(policyData);
+ policyData = new CreateFirewallController().setDataToPolicyRestAdapter(policyData);
}else if("Micro Service".equalsIgnoreCase(policyData.getConfigPolicyType())){
- CreateDcaeMicroServiceController msController = new CreateDcaeMicroServiceController();
- policyData = msController.setDataToPolicyRestAdapter(policyData, root);
+ policyData = new CreateDcaeMicroServiceController().setDataToPolicyRestAdapter(policyData, root);
}
}
diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/ActionPolicyController.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/ActionPolicyController.java
index 88cb2e2..a556bee 100644
--- a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/ActionPolicyController.java
+++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/ActionPolicyController.java
@@ -26,6 +26,7 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import javax.xml.bind.JAXBElement;
@@ -166,8 +167,9 @@
AttributeValueType attributeValue = (AttributeValueType) attributeAssignmentExpression
.getExpression().getValue();
if (attributeID.equals(PERFORMER_ATTRIBUTEID)) {
- for (String key : performer.keySet()) {
- String keyValue = performer.get(key);
+ for ( Entry<String, String> entry: performer.entrySet()) {
+ String key = entry.getKey();
+ String keyValue = entry.getValue();
if (keyValue.equals(attributeValue.getContent().get(0))) {
policyAdapter.setActionPerformer(key);
}
@@ -235,10 +237,9 @@
ruleMap.put("id", "A" + (index + 1));
// Populate combo box
Map<String, String> dropDownMap = PolicyController.getDropDownMap();
- for (String key : dropDownMap.keySet()) {
- String keyValue = dropDownMap.get(key);
- if (keyValue.equals(actionApply.getFunctionId())) {
- ruleMap.put("dynamicRuleAlgorithmCombo", key);
+ for ( Entry<String, String> entry : dropDownMap.entrySet()) {
+ if (entry.getValue().equals(actionApply.getFunctionId())) {
+ ruleMap.put("dynamicRuleAlgorithmCombo", entry.getKey());
}
}
// Populate the key and value fields
diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateDcaeMicroServiceController.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateDcaeMicroServiceController.java
index 5d626ae..e2db78c 100644
--- a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateDcaeMicroServiceController.java
+++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateDcaeMicroServiceController.java
@@ -433,7 +433,7 @@
return settings;
}
- public Map<String, String> load(byte[] source) throws IOException {
+ public Map<String, String> load(byte[] source) {
Yaml yaml = new Yaml();
@SuppressWarnings("unchecked")
Map<Object, Object> yamlMap = (Map<Object, Object>) yaml.load(Arrays.toString(source));
@@ -831,7 +831,9 @@
ObjectNode node = nodeFactory.objectNode();
String prevKey = null;
String presKey;
- for(String key: element.keySet()){
+ for(Entry<String, String> entry: element.entrySet()){
+ String key = entry.getKey();
+ String value = entry.getValue();
if(key.contains(".")){
presKey = key.substring(0,key.indexOf('.'));
}else if(key.contains("@")){
@@ -859,7 +861,7 @@
nodeKey = key.substring(0,key.indexOf('.'));
}
if(nodeKey.equals(key.substring(0,key.indexOf('.')))){
- node.put(key.substring(key.indexOf('.')+1), element.get(key));
+ node.put(key.substring(key.indexOf('.')+1), value);
}else{
if(node.size()!=0){
if(nodeKey.contains("@")){
@@ -888,7 +890,7 @@
if(nodeKey.contains("@")){
arryKey = nodeKey.substring(0,nodeKey.indexOf('@'));
}
- node.put(key.substring(key.indexOf('.')+1), element.get(key));
+ node.put(key.substring(key.indexOf('.')+1), value);
}
}else if(node.size()!=0){
if(nodeKey.contains("@")){
@@ -926,14 +928,14 @@
oldValue = key.substring(0,key.indexOf('@'));
}
if(oldValue.equals(key.substring(0,key.indexOf('@')))){
- jsonArray.put(element.get(key));
+ jsonArray.put(value);
}else{
jsonResult.put(oldValue, jsonArray);
jsonArray = new JSONArray();
}
oldValue = key.substring(0,key.indexOf('@'));
}else{
- jsonResult.put(key, element.get(key));
+ jsonResult.put(key, value);
}
}else{
if(key.contains("@")){
@@ -952,14 +954,14 @@
oldValue = key.substring(0,key.indexOf('@'));
}
if(oldValue.equals(key.substring(0,key.indexOf('@')))){
- jsonArray.put(element.get(key));
+ jsonArray.put(value);
}else{
jsonResult.put(oldValue, jsonArray);
jsonArray = new JSONArray();
}
oldValue = key.substring(0,key.indexOf('@'));
}else{
- jsonResult.put(key, element.get(key));
+ jsonResult.put(key, value);
}
}
}
@@ -1459,8 +1461,8 @@
if(value instanceof LinkedHashMap<?, ?>){
LinkedHashMap<String, Object> secondObjec = new LinkedHashMap<>();
readRecursivlyJSONContent((LinkedHashMap<String, ?>) value, secondObjec);
- for(String objKey: secondObjec.keySet()){
- data.put(key+"." +objKey, secondObjec.get(objKey));
+ for( Entry<String, Object> entry : secondObjec.entrySet()){
+ data.put(key+"." + entry.getKey(), entry.getValue());
}
}else if(value instanceof ArrayList){
ArrayList<?> jsonArrayVal = (ArrayList<?>)value;
@@ -1469,8 +1471,8 @@
if(arrayvalue instanceof LinkedHashMap<?, ?>){
LinkedHashMap<String, Object> newData = new LinkedHashMap<>();
readRecursivlyJSONContent((LinkedHashMap<String, ?>) arrayvalue, newData);
- for(String objKey: newData.keySet()){
- data.put(key+"@"+i+"." +objKey, newData.get(objKey));
+ for(Entry<String, Object> entry: newData.entrySet()){
+ data.put(key+"@"+i+"." +entry.getKey(), entry.getValue());
}
}else if(arrayvalue instanceof ArrayList){
ArrayList<?> jsonArrayVal1 = (ArrayList<?>)value;
diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateFirewallController.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateFirewallController.java
index b4a8fd8..2750ff5 100644
--- a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateFirewallController.java
+++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateFirewallController.java
@@ -336,7 +336,7 @@
String ruleDestPort;
String ruleAction;
List <String> valueDesc;
- StringBuffer displayString = new StringBuffer();
+ StringBuilder displayString = new StringBuilder();
for (String id : termCollectorList) {
List<Object> tmList = commonClassDao.getDataById(TermList.class, "termName", id);
jpaTermList = (TermList) tmList.get(0);
@@ -349,15 +349,17 @@
if(srcList.startsWith(GROUP)){
AddressGroup ag;
ag= mappingAddressGroup(srcList);
- displayString.append("\n\t"+"Group has :"+ag.getPrefixList()+"\n");
- for(String groupItems:ag.getPrefixList().split(",")){
- valueDesc=mapping(groupItems);
- displayString.append("\n\t"+"Name: "+groupItems);
- if(!valueDesc.isEmpty()){
- displayString.append("\n\t"+"Description: "+valueDesc.get(1));
- displayString.append("\n\t"+"Value: "+valueDesc.get(0));
+ displayString.append("\n\t"+"Group has :"+(ag != null ? ag.getPrefixList() : "") +"\n");
+ if (ag != null) {
+ for(String groupItems:ag.getPrefixList().split(",")){
+ valueDesc=mapping(groupItems);
+ displayString.append("\n\t"+"Name: "+groupItems);
+ if(!valueDesc.isEmpty()){
+ displayString.append("\n\t"+"Description: "+valueDesc.get(1));
+ displayString.append("\n\t"+"Value: "+valueDesc.get(0));
+ }
+ displayString.append("\n");
}
- displayString.append("\n");
}
}else{
if(!srcList.equals(ANY)){
@@ -379,13 +381,15 @@
if(destList.startsWith(GROUP)){
AddressGroup ag;
ag= mappingAddressGroup(destList);
- displayString.append("\n\t"+"Group has :"+ag.getPrefixList()+"\n");
- for(String groupItems:ag.getPrefixList().split(",")){
- valueDesc=mapping(groupItems);
- displayString.append("\n\t"+"Name: "+groupItems);
- displayString.append("\n\t"+"Description: "+valueDesc.get(1));
- displayString.append("\n\t"+"Value: "+valueDesc.get(0));
- displayString.append("\n\t");
+ displayString.append("\n\t"+"Group has :"+ (ag != null ? ag.getPrefixList() : "") +"\n");
+ if (ag != null) {
+ for(String groupItems:ag.getPrefixList().split(",")){
+ valueDesc=mapping(groupItems);
+ displayString.append("\n\t"+"Name: "+groupItems);
+ displayString.append("\n\t"+"Description: "+valueDesc.get(1));
+ displayString.append("\n\t"+"Value: "+valueDesc.get(0));
+ displayString.append("\n\t");
+ }
}
}else{
if(!destList.equals(ANY)){
@@ -416,19 +420,21 @@
if(destServices.startsWith(GROUP)){
GroupServiceList sg;
sg= mappingServiceGroup(destServices);
- displayString.append("\n\t"+"Service Group has :"+sg.getServiceList()+"\n");
- for(String groupItems:sg.getServiceList().split(",")){
- ServiceList sl;
- sl= mappingServiceList(groupItems);
- displayString.append("\n\t"+"Name: "+
- sl.getServiceName());
- displayString.append("\n\t"+"Description: "+
- sl.getServiceDescription());
- displayString.append("\n\t"+"Transport-Protocol: "+
- sl.getServiceTransProtocol());
- displayString.append("\n\t"+"Ports: "+
- sl.getServicePorts());
- displayString.append("\n");
+ displayString.append("\n\t"+"Service Group has :"+ (sg != null ? sg.getServiceList() : "") +"\n");
+ if (sg != null) {
+ for(String groupItems:sg.getServiceList().split(",")){
+ ServiceList sl;
+ sl= mappingServiceList(groupItems);
+ displayString.append("\n\t"+"Name: "+
+ sl.getServiceName());
+ displayString.append("\n\t"+"Description: "+
+ sl.getServiceDescription());
+ displayString.append("\n\t"+"Transport-Protocol: "+
+ sl.getServiceTransProtocol());
+ displayString.append("\n\t"+"Ports: "+
+ sl.getServicePorts());
+ displayString.append("\n");
+ }
}
}
else{
diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/DecisionPolicyController.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/DecisionPolicyController.java
index 49496cd..6f8eea8 100644
--- a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/DecisionPolicyController.java
+++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/DecisionPolicyController.java
@@ -27,6 +27,7 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import javax.xml.bind.JAXBElement;
@@ -283,10 +284,9 @@
Map<String, String> ruleMap = new HashMap<>();
ruleMap.put("id", "A" + (index +1));
Map<String, String> dropDownMap = PolicyController.getDropDownMap();
- for (String key : dropDownMap.keySet()) {
- String keyValue = dropDownMap.get(key);
- if (keyValue.equals(decisionApply.getFunctionId())) {
- ruleMap.put("dynamicRuleAlgorithmCombo", key);
+ for (Entry<String, String> entry : dropDownMap.entrySet()) {
+ if (entry.getValue().equals(decisionApply.getFunctionId())) {
+ ruleMap.put("dynamicRuleAlgorithmCombo", entry.getKey());
}
}
// Populate the key and value fields
diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PolicyController.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PolicyController.java
index 03709cc..2df70c7 100644
--- a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PolicyController.java
+++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PolicyController.java
@@ -29,6 +29,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Properties;
import javax.annotation.PostConstruct;
@@ -242,8 +243,8 @@
//Initialize the FunctionDefinition table at Server Start up
Map<Datatype, List<FunctionDefinition>> functionMap = getFunctionDatatypeMap();
- for (Datatype id : functionMap.keySet()) {
- List<FunctionDefinition> functionDefinations = functionMap.get(id);
+ for ( Entry<Datatype, List<FunctionDefinition>> entry : functionMap.entrySet()) {
+ List<FunctionDefinition> functionDefinations = entry.getValue();
for (FunctionDefinition functionDef : functionDefinations) {
dropDownMap.put(functionDef.getShortname(),functionDef.getXacmlid());
}
diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/daoImp/CommonClassDaoImpl.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/daoImp/CommonClassDaoImpl.java
index b9eb5ed..170c308 100644
--- a/POLICY-SDK-APP/src/main/java/org/onap/policy/daoImp/CommonClassDaoImpl.java
+++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/daoImp/CommonClassDaoImpl.java
@@ -21,6 +21,7 @@
package org.onap.policy.daoImp;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -253,7 +254,7 @@
@Override
public List<Object> checkExistingGroupListforUpdate(String arg0, String arg1) {
- return null;
+ return Collections.emptyList();
}
diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/utils/ConfigurableRESTUtils.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/utils/ConfigurableRESTUtils.java
index 3e935dd..7457610 100644
--- a/POLICY-SDK-APP/src/main/java/org/onap/policy/utils/ConfigurableRESTUtils.java
+++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/utils/ConfigurableRESTUtils.java
@@ -27,6 +27,7 @@
import java.net.HttpURLConnection;
import java.net.URL;
import java.util.Map;
+import java.util.Map.Entry;
import org.onap.policy.common.logging.flexlogger.FlexLogger;
import org.onap.policy.common.logging.flexlogger.Logger;
@@ -88,11 +89,9 @@
connection.setUseCaches(false);
// add hard-coded headers
- for (String headerName : hardCodedHeaderMap.keySet()) {
- connection.addRequestProperty(headerName, hardCodedHeaderMap.get(headerName));
+ for (Entry<String, String> entry : hardCodedHeaderMap.entrySet()) {
+ connection.addRequestProperty(entry.getKey(), entry.getValue());
}
-
-
if (jsonBody != null){
connection.setDoInput(true);