From d744207d9c6d6fdc51495d70f5ca62b6ca835930 Mon Sep 17 00:00:00 2001
From: Eddie Groh <groh@fortiss.org>
Date: Tue, 12 Sep 2023 14:31:03 +0200
Subject: [PATCH] Refactored code for better maintainability

Issue-Ref: 4310
Issue-Url: https://git.fortiss.org/af3/af3/-/issues/4310

Signed-off-by: Eddie Groh <groh@fortiss.org>
---
 .../tooling/ext/quality/ui/view/fx/.ratings   |   2 +-
 .../ui/view/fx/ModelQualityFXController.java  |  33 ++--
 .../org/fortiss/tooling/ext/quality/.ratings  |   2 +-
 .../ext/quality/GraphMetricsProvider.java     | 155 +++++++++++-------
 .../tooling/ext/quality/storage/.ratings      |   2 +-
 .../ext/quality/storage/CSVFileWriter.java    |  17 ++
 6 files changed, 136 insertions(+), 75 deletions(-)

diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/.ratings b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/.ratings
index 5bbde6cd3..7bcec2aad 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/.ratings
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/.ratings
@@ -1,3 +1,3 @@
 IModelQualityViewPart.java 708f8089645df12098ea67190805cce343045d2e YELLOW
-ModelQualityFXController.java 391dc121678e1741b8ee98f90de18996ba2c37b2 YELLOW
+ModelQualityFXController.java 4272d7043944effc9532d4a122fd50b63b79fb82 YELLOW
 ModelQualityFXViewPart.java 179abf18d6e3b6c844076620f53b43ac8a42c217 YELLOW
diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXController.java b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXController.java
index 391dc1216..4272d7043 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXController.java
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXController.java
@@ -122,7 +122,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 		EObject selected = checkAndPickFirst(selection, EObject.class);
 
 		if(selected == null) {
-			updateCharts();
+			updateCharts("Nothing selected");
 			return;
 		}
 
@@ -131,30 +131,35 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 						: getFirstParentWithType(selected, IProjectRootElement.class);
 
 		if(currentRootElement == null) {
-			updateCharts();
+			updateCharts("Selection does not have a ProjectRoot");
 			return;
 		}
 
 		var root_provider = manager.getRootNodesMap().get(currentRootElement);
 
 		if(root_provider == null) {
-			updateCharts();
+			updateCharts("Currently no metrics available, try generating them");
 			return;
 		}
 
 		lastSelectedElement = selected;
-		updateCharts();
+		updateCharts("Selection does not have metric information");
 	}
 
 	/**
 	 * Is triggered when the choice box changes.
 	 */
 	public void onChoiceBoxChange() {
-		updateCharts();
+		updateCharts("Invalid selection or metrics not available");
 	}
 
-	/** Updates the chart in the metric view. */
-	private void updateCharts() {
+	/**
+	 * Updates the chart in the metric view.
+	 * 
+	 * @param message
+	 *            to display if metric is not available
+	 */
+	private void updateCharts(String message) {
 
 		if(lastSelectedElement != null) {
 			MetricDataManager manager =
@@ -187,13 +192,16 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 
 						// Create PieChart and add data
 						pieChart.getData().clear();
+						pieChart.setLegendVisible(false);
+						pieChart.setTitle("");
+						DecimalFormat format = new DecimalFormat("0.#");
+
 						for(var child : node.getChildren()) {
 							Double value = child.getValueAsDouble(choiceBox.getValue());
 							if(value != null) {
-								PieChart.Data slice = new PieChart.Data(child.getName(), value);
+								PieChart.Data slice = new PieChart.Data(
+										child.getName() + ": " + format.format(value), value);
 								pieChart.getData().add(slice);
-								pieChart.setLegendVisible(false);
-								pieChart.setTitle("");
 							}
 						}
 					}
@@ -221,9 +229,12 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 					// Return, otherwise the data will be cleared
 					return;
 				}
+				message = "Selection does not have contained elements";
+			} else {
+				message = "Selection does not have metric information";
 			}
 		}
-		borderPane.setCenter(new Label("No metrics available for this selection"));
+		borderPane.setCenter(new Label(message));
 		pieChart.getData().clear();
 	}
 
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/.ratings b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/.ratings
index f6e940178..b9ac9906a 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/.ratings
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/.ratings
@@ -1,4 +1,4 @@
 AF3QualityActivator.java 353c3d99f423997e4e99a896b3c095fd77d81431 YELLOW
-GraphMetricsProvider.java 2d3fff61af2537bc5b10b3c210dc8e4b3ad49c1d YELLOW
+GraphMetricsProvider.java d2d0db1d00adeffeeb55b881a2ed81b7107cdbe2 YELLOW
 HierarchicElementProvider.java 3c114abd47ecc0d7da751c3f05c83fd49a30717b YELLOW
 IMetricProvider.java 5e3d0debc0f81ed4e6c7dbc0f8d0e55df1bfde39 YELLOW
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/GraphMetricsProvider.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/GraphMetricsProvider.java
index 2d3fff61a..d2d0db1d0 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/GraphMetricsProvider.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/GraphMetricsProvider.java
@@ -227,71 +227,23 @@ public class GraphMetricsProvider {
 
 				// Initialize all variables
 				Stack<IHierarchicElement> stack = new Stack<>();
-				Map<IHierarchicElement, List<IHierarchicElement>> pred = new HashMap<>();
-				Map<IHierarchicElement, Integer> dist = new HashMap<>();
-				Map<IHierarchicElement, Double> sigma = new HashMap<>();
+				Map<IHierarchicElement, List<IHierarchicElement>> predecessors = new HashMap<>();
+				Map<IHierarchicElement, Integer> distance = new HashMap<>();
+				Map<IHierarchicElement, Double> dependency = new HashMap<>();
 				Queue<IHierarchicElement> queue = new LinkedList<>();
 
 				for(IHierarchicElement v : graphNodes) {
-					pred.put(v, new ArrayList<>());
-					dist.put(v, -1);
-					sigma.put(v, 0.0);
+					predecessors.put(v, new ArrayList<>());
+					distance.put(v, -1);
+					dependency.put(v, 0.0);
 				}
-				sigma.put(elementNode, 1.0);
-				dist.put(elementNode, 0);
+				dependency.put(elementNode, 1.0);
+				distance.put(elementNode, 0);
 				queue.add(elementNode);
 
-				while(!queue.isEmpty()) {
-					IHierarchicElement v = queue.remove();
-					stack.push(v);
-
-					// Iterate over all connectors going out from the element
-					for(ExitConnectorBase exitConnectors : EcoreUtils
-							.pickInstanceOf(ExitConnectorBase.class, v.getConnectors())) {
-						// Iterate over all connections
-						for(IConnection outgoingConnection : exitConnectors.getOutgoing()) {
-
-							Set<IHierarchicElement> targetElements = new HashSet<>();
-							if(recursively) {
-								recursivlyFollowOutgoingConnection(outgoingConnection.getTarget(),
-										targetElements);
-							} else {
-								IHierarchicElement primaryTargetElement =
-										outgoingConnection.getTarget().getOwner();
-								// When we have a connection to the scopeElement, we have a
-								// connection leaving the local scope
-								if(scopeElement == primaryTargetElement) {
-									// Continue searching for outgoing connections on the target
-									// Connector
-									IConnector targetConnector = outgoingConnection.getTarget();
-									for(IConnection secondaryOutgoingConnection : targetConnector
-											.getOutgoing()) {
-										targetElements.add(
-												secondaryOutgoingConnection.getTarget().getOwner());
-									}
-								} else {
-									targetElements.add(primaryTargetElement);
-								}
-							}
-							for(IHierarchicElement targetElement : targetElements) {
-
-								// Check if element is in node list, to avoid creating data for
-								// nodes outside of our graph
-								if(graphNodes.contains(targetElement)) {
-									if(dist.get(targetElement) < 0) {
-										queue.add(targetElement);
-										dist.put(targetElement, dist.get(v) + 1);
-									}
-									if(dist.get(targetElement) == dist.get(v) + 1) {
-										sigma.put(targetElement,
-												sigma.get(targetElement) + sigma.get(v));
-										pred.get(targetElement).add(v);
-									}
-								}
-							}
-						}
-					}
-				}
+				processCentrality(queue, stack, predecessors, distance, dependency, graphNodes,
+						elementNode, recursively);
+
 				// Initialize delta
 				Map<IHierarchicElement, Double> delta = new HashMap<>();
 				for(IHierarchicElement v : graphNodes) {
@@ -300,8 +252,8 @@ public class GraphMetricsProvider {
 
 				while(!stack.isEmpty()) {
 					IHierarchicElement w = stack.pop();
-					for(IHierarchicElement v : pred.get(w)) {
-						double c = (sigma.get(v) / sigma.get(w)) * (1.0 + delta.get(w));
+					for(IHierarchicElement v : predecessors.get(w)) {
+						double c = (dependency.get(v) / dependency.get(w)) * (1.0 + delta.get(w));
 						delta.put(v, delta.get(v) + c);
 					}
 					if(!w.equals(elementNode)) {
@@ -318,4 +270,85 @@ public class GraphMetricsProvider {
 			}
 		}
 	}
+
+	/**
+	 * Performs intermediate calculations for the betweenness centrality algorithm
+	 * 
+	 * @param queue
+	 *            of nodes for Breadth-First Search.
+	 * @param stack
+	 *            of nodes for Breadth-First Search.
+	 * @param predecessor
+	 *            maps to lists used to keep track of the predecessors of nodes in the shortest
+	 *            paths.
+	 * @param distance
+	 *            map used to store the shortest distances from the source node to other nodes.
+	 * @param dependency
+	 *            map used to calculate and store the dependency values for nodes in the network.
+	 *            These values are part of the calculation for betweenness centrality.
+	 * @param graphNodes
+	 *            list of nodes
+	 * @param scopeElement
+	 *            the scope of this calculation
+	 * @param recursively
+	 *            if true, all calculation will be carried out on leaf elements
+	 */
+	private static void processCentrality(Queue<IHierarchicElement> queue,
+			Stack<IHierarchicElement> stack,
+			Map<IHierarchicElement, List<IHierarchicElement>> predecessor,
+			Map<IHierarchicElement, Integer> distance, Map<IHierarchicElement, Double> dependency,
+			Set<IHierarchicElement> graphNodes, IHierarchicElement scopeElement,
+			boolean recursively) {
+		while(!queue.isEmpty()) {
+			IHierarchicElement v = queue.remove();
+			stack.push(v);
+
+			// Iterate over all connectors going out from the element
+			for(ExitConnectorBase exitConnectors : EcoreUtils
+					.pickInstanceOf(ExitConnectorBase.class, v.getConnectors())) {
+				// Iterate over all connections
+				for(IConnection outgoingConnection : exitConnectors.getOutgoing()) {
+
+					Set<IHierarchicElement> targetElements = new HashSet<>();
+					if(recursively) {
+						recursivlyFollowOutgoingConnection(outgoingConnection.getTarget(),
+								targetElements);
+					} else {
+						IHierarchicElement primaryTargetElement =
+								outgoingConnection.getTarget().getOwner();
+						// When we have a connection to the scopeElement, we have a
+						// connection leaving the local scope
+						if(scopeElement == primaryTargetElement) {
+							// Continue searching for outgoing connections on the target
+							// Connector
+							IConnector targetConnector = outgoingConnection.getTarget();
+							for(IConnection secondaryOutgoingConnection : targetConnector
+									.getOutgoing()) {
+								targetElements
+										.add(secondaryOutgoingConnection.getTarget().getOwner());
+							}
+						} else {
+							targetElements.add(primaryTargetElement);
+						}
+					}
+					for(IHierarchicElement targetElement : targetElements) {
+
+						// Check if element is in node list, to avoid creating data for
+						// nodes outside of our graph
+						if(graphNodes.contains(targetElement)) {
+							if(distance.get(targetElement) < 0) {
+								queue.add(targetElement);
+								distance.put(targetElement, distance.get(v) + 1);
+							}
+							if(distance.get(targetElement) == distance.get(v) + 1) {
+								dependency.put(targetElement,
+										dependency.get(targetElement) + dependency.get(v));
+								predecessor.get(targetElement).add(v);
+							}
+						}
+					}
+				}
+			}
+		}
+	}
 }
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/.ratings b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/.ratings
index 10d1f9061..d54c7aa59 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/.ratings
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/.ratings
@@ -1,2 +1,2 @@
-CSVFileWriter.java 4e079908d3dd9af600ef97773f795c3e6a726c4b YELLOW
+CSVFileWriter.java 3798d6cfd828cf103885c168066853b28fac85f7 YELLOW
 ModelQualityStorageManager.java 124faddb3d297d23448c15379446cc847a2790a9 YELLOW
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/CSVFileWriter.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/CSVFileWriter.java
index 4e079908d..3798d6cfd 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/CSVFileWriter.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/CSVFileWriter.java
@@ -86,8 +86,25 @@ public class CSVFileWriter {
 			} catch(IOException e) {
 				e.printStackTrace();
 			}
+			writeMetricsToFile(map, allKeys, path, createNewIndex);
 		}
+	}
 
+	/**
+	 * Function to write the specified metrics into a csv file.
+	 * 
+	 * 
+	 * @param map
+	 *            of the metrics which are written into the file
+	 * @param allKeys
+	 *            a list of keys which will be extracted from the provided map
+	 * @param path
+	 *            location of the file
+	 * @param createNewIndex
+	 *            if a index should be written before writing any values
+	 */
+	private static void writeMetricsToFile(Map<IProjectRootElement, MetricTreeNode> map,
+			List<String> allKeys, Path path, boolean createNewIndex) {
 		try(var writer = new BufferedWriter(new FileWriter(path.toFile(), true))) {
 
 			if(createNewIndex) {
-- 
GitLab