From c32b0dcd86a2534e438f25a85ee6e26b7c143529 Mon Sep 17 00:00:00 2001
From: Simon Barner <barner@fortiss.org>
Date: Thu, 12 Oct 2023 11:02:31 +0200
Subject: [PATCH] Initial code rating for tooling.ext.quality.ui (RED, GREEN)

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

Signed-off-by: Simon Barner <barner@fortiss.org>
---
 .../html/developer/.ratings                   |  2 +-
 .../fortiss/tooling/ext/quality/ui/.ratings   |  4 +-
 .../ui/ModelQualityExtractionMenu.java        |  5 +-
 .../tooling/ext/quality/ui/view/fx/.ratings   |  6 +-
 .../ui/view/fx/ModelQualityFXController.java  | 70 +++++++++----------
 .../ui/view/fx/ModelQualityFXViewPart.java    |  7 +-
 6 files changed, 46 insertions(+), 48 deletions(-)

diff --git a/org.fortiss.tooling.ext.quality.ui/html/developer/.ratings b/org.fortiss.tooling.ext.quality.ui/html/developer/.ratings
index 91f80958f..1c6ee9d1b 100644
--- a/org.fortiss.tooling.ext.quality.ui/html/developer/.ratings
+++ b/org.fortiss.tooling.ext.quality.ui/html/developer/.ratings
@@ -1 +1 @@
-documentation.html aaa534ab30ef337c5e7bc75d37490eb6a5a4edd1 YELLOW
+documentation.html aaa534ab30ef337c5e7bc75d37490eb6a5a4edd1 GREEN
diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/.ratings b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/.ratings
index e5e3fec61..4f312f29e 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/.ratings
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/.ratings
@@ -1,2 +1,2 @@
-ModelQualityExtractionMenu.java 87cdfa5d4f939cc9f21afc8d6361a2089f440f20 YELLOW
-ModelQualityUIActivator.java 2f47db82bc088072dbfe53f82d3ed417d77fd23c YELLOW
+ModelQualityExtractionMenu.java e2926b47f4f30adb160c47aef773853bb5b03fcb RED
+ModelQualityUIActivator.java 2f47db82bc088072dbfe53f82d3ed417d77fd23c GREEN
diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/ModelQualityExtractionMenu.java b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/ModelQualityExtractionMenu.java
index 87cdfa5d4..e2926b47f 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/ModelQualityExtractionMenu.java
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/ModelQualityExtractionMenu.java
@@ -35,7 +35,7 @@ import org.fortiss.tooling.kernel.ui.extension.IContextMenuContributor;
 import org.fortiss.tooling.kernel.ui.extension.data.ContextMenuContextProvider;
 
 /**
- * Creates a context menu entry to extract metrics for the selected {@link FileProject}.
+ * Creates a context menu entry to extract metrics for the selected model.
  * 
  * @author groh
  */
@@ -48,6 +48,9 @@ public class ModelQualityExtractionMenu implements IContextMenuContributor {
 	@Override
 	public List<IContributionItem> getContributedItems(EObject selection,
 			ContextMenuContextProvider contextProvider) {
+		// TODO (SB): Use of FileProject must be avoided
+		// Change the condition to "selection.eContainer() != null" (only FileProjects)
+		// have a null parent and remove dependency to org.fortiss.af3.project from this plugin
 		if(!(selection instanceof FileProject)) {
 			return emptyList();
 		}
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 99d0ae977..1b3e6d886 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 24b571533dfa46cb33e5aef49ac91a48a92c9708 YELLOW
-ModelQualityFXViewPart.java 179abf18d6e3b6c844076620f53b43ac8a42c217 YELLOW
+IModelQualityViewPart.java 708f8089645df12098ea67190805cce343045d2e GREEN
+ModelQualityFXController.java f4ae42467509d29326bdabe822310083253910dc RED
+ModelQualityFXViewPart.java 4f142d1b04ab9e5f0ce54ff248fe0abf6f141937 RED
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 24b571533..f4ae42467 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
@@ -84,16 +84,15 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 
 	/**
 	 * Selection of colors for the overlays in the spider chart. The length of this array also
-	 * limits
-	 * the depth of the {@link ModelQualityFXController#lastViewedElements} list
+	 * limits the depth of the {@link ModelQualityFXController#lastViewedElements} list.
 	 */
 	private static Color[] SPIDER_CHART_OVERLAY_COLORS = {Color.GREEN, Color.BLUE, Color.RED};
 
-	/** ChoiceBox for selecting the metric which shall be displayed. */
+	/** {@link ChoiceBox} for selecting the metric which shall be displayed. */
 	@FXML
 	private ChoiceBox<MetricKey> childMetricChoiceBox;
 
-	/** PieChart displaying the metrics. */
+	/** {@link PieChart} displaying the metrics. */
 	@FXML
 	private PieChart pieChart;
 
@@ -101,19 +100,19 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 	@FXML
 	private BorderPane borderPane;
 
-	/** ChoiceBox for selecting the metric displayed on the x axis of the scatter chart. */
+	/** {@link ChoiceBox} for selecting the metric displayed on the x axis of the scatter chart. */
 	@FXML
 	private ChoiceBox<MetricKey> xScatterMetricChoiceBox;
 
-	/** ChoiceBox for selecting the metric displayed on the y axis of the scatter chart. */
+	/** {@link ChoiceBox} for selecting the metric displayed on the y axis of the scatter chart. */
 	@FXML
 	private ChoiceBox<MetricKey> yScatterMetricChoiceBox;
 
-	/** A scatter chart for displaying metrics. */
+	/** A {@link ScatterChart} for displaying metrics. */
 	@FXML
 	private ScatterChart<Number, Number> scatterChart;
 
-	/** A bar chart for displaying metrics. */
+	/** A {@link BarChart} for displaying metrics. */
 	@FXML
 	private BarChart<String, Number> barChart;
 
@@ -130,7 +129,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 	 */
 	private EObject lastSelectedElement;
 
-	/** List of elements for which metrics were displayed */
+	/** List of elements for which metrics were displayed. */
 	private List<Pair<MetricTreeNode, Set<MetricKey>>> lastViewedElements = new ArrayList<>();
 
 	/** {@inheritDoc} */
@@ -152,13 +151,16 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 		// Invalidate last selection, so if no new selection is found the charts are cleared
 		lastSelectedElement = null;
 
+		// TODO (SB): Unneeded?
 		ModelQualityService.getInstance();
+
+		// TODO (SB): Use IModelQualityService
 		var manager = ModelQualityService.getInstance().getMetricDataManagerInstance();
 
 		EObject selected = checkAndPickFirst(selection, EObject.class);
 
 		if(selected == null) {
-			updateCharts("Nothing selected");
+			updateCharts("Nothing selected.");
 			return;
 		}
 
@@ -167,7 +169,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 						: getFirstParentWithType(selected, IProjectRootElement.class);
 
 		if(currentRootElement == null) {
-			updateCharts("Selection does not have a ProjectRoot");
+			updateCharts("Selection does not have a project root element.");
 			return;
 		}
 
@@ -175,34 +177,30 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 
 		if(rootDataElement == null) {
 			bottomText.setText(
-					"Save model or select \"Export Metrics\" in project options to generate metrics");
+					"Save model or select \"Export Metrics\" in project options to generate metrics.");
 			updateCharts("No metrics available");
 			return;
 		}
 
 		bottomText.setText("Metrics last updated: " +
-				timeFormat.format(rootDataElement.getLastRefresh()) + ". Save model to refresh");
+				timeFormat.format(rootDataElement.getLastRefresh()) + ". Save model to refresh.");
 		lastSelectedElement = selected;
-		updateCharts("Selection does not have metric information");
+		updateCharts("Selection does not have metric information.");
 	}
 
-	/** Is triggered when the choice box changes. */
+	/** Triggered when the choice box changes. */
 	public void onChoiceBoxChange() {
-		updateCharts("Invalid selection or metrics not available");
+		updateCharts("Invalid selection or metrics not available.");
 	}
 
-	/**
-	 * Is triggered when the child metric choice box changes.
-	 */
+	/** Triggered when the child metric choice box changes. */
 	public void onChildMetricChoiceBoxChange() {
-		updateCharts("Invalid selection or metrics not available");
+		updateCharts("Invalid selection or metrics not available.");
 	}
 
-	/**
-	 * Is triggered when one of the scatter choice boxes changes.
-	 */
+	/** Triggered when one of the scatter choice boxes changes. */
 	public void onScatterMetricChoiceBoxChange() {
-		updateCharts("Invalid selection or metrics not available");
+		updateCharts("Invalid selection or metrics not available.");
 	}
 
 	/**
@@ -235,9 +233,9 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 					// Return, otherwise the data will be cleared
 					return;
 				}
-				message = "Selection does not have contained elements";
+				message = "Selection does not have contained elements.";
 			} else {
-				message = "Selection does not have metric information";
+				message = "Selection does not have metric information.";
 			}
 		} else {
 			// Nothing selected, so remove the bottom text
@@ -248,7 +246,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 	}
 
 	/**
-	 * Updates the {@link #pieChart} with data from the provided {@link MetricTreeNode}
+	 * Updates the {@link #pieChart} with data from the provided {@link MetricTreeNode}.
 	 * 
 	 * @param node
 	 *            which provides the data for this chart
@@ -275,7 +273,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 	}
 
 	/**
-	 * Updates the {@link #scatterChart} with data from the provided {@link MetricTreeNode}
+	 * Updates the {@link #scatterChart} with data from the provided {@link MetricTreeNode}.
 	 * 
 	 * @param node
 	 *            which provides the data for this chart
@@ -287,11 +285,9 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 
 			scatterChart.getXAxis().setLabel(xScatterKey.toString());
 			scatterChart.getYAxis().setLabel(yScatterKey.toString());
-
 			scatterChart.getData().clear();
 
 			for(var child : node.getChildren()) {
-
 				Double xValue = child.getValueAsDouble(xScatterKey);
 				Double yValue = child.getValueAsDouble(yScatterKey);
 
@@ -307,7 +303,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 	}
 
 	/**
-	 * Updates the {@link #barChart} with data from the provided {@link MetricTreeNode}
+	 * Updates the {@link #barChart} with data from the provided {@link MetricTreeNode}.
 	 * 
 	 * @param node
 	 *            which provides the data for this chart
@@ -339,7 +335,6 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 	 * elements in the list is created, and then these elements are overlaid.
 	 */
 	private SpiderChartViewer buildSpiderChart() {
-
 		// Calculate the intersection of the keys, i.e. a set of keys which have a non-null value in
 		// all elements of the lastViewedElements list
 		Set<MetricKey> keysIntersection = lastViewedElements.get(0).getValue();
@@ -348,6 +343,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 		}
 
 		// Create DataSeries for all elements
+		// TODO (SB): Use static import for toList()
 		List<Pair<MetricTreeNode, DataSeries>> dataSeriesList = lastViewedElements.stream()
 				.map(pair -> new Pair<>(pair.getKey(), new DataSeries(pair.getKey().getName())))
 				.collect(Collectors.toList());
@@ -362,7 +358,6 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 
 		// Iterate over all keys, and add data points
 		for(MetricKey key : keysIntersection) {
-
 			// Calculate maximumValue for the range of the axis
 			Double maxmiumValue = lastViewedElements.stream()
 					.mapToDouble(p -> p.getKey().getValueAsDouble(key)).max().getAsDouble();
@@ -386,6 +381,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 		}
 
 		// Iterate over all dataSeries and add them to the spider chart
+		// TODO (SB): Iterate over collection. get(i) is expensive. "for(var pair : dataSeriesList)"
 		for(int i = 0; i < dataSeriesList.size(); i++) {
 			var pair = dataSeriesList.get(i);
 			spiderChart.addData(pair.getValue());
@@ -421,17 +417,19 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
 				return;
 			}
 
+			// Store IModelQualityService.getInstance() (sic!) in a variable (e.g. "mqs") for
+			// readability
 			var rootDataElement = ModelQualityService.getInstance().getMetricDataManagerInstance()
 					.getDataRootElementMap().get(updatedElement);
 			if(rootDataElement != null) {
 				bottomText.setText("Metrics last updated: " +
 						timeFormat.format(rootDataElement.getLastRefresh()) +
-						". Save model to refresh");
+						". Save model to refresh.");
 			} else {
 				bottomText.setText(
-						"Save model or select \"Export Metrics\" in project options to generate metrics");
+						"Save model or select \"Export Metrics\" in project options to generate metrics.");
 			}
-			updateCharts("Selection does not have metric information");
+			updateCharts("Selection does not have metric information.");
 		});
 	}
 
diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXViewPart.java b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXViewPart.java
index 179abf18d..4f142d1b0 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXViewPart.java
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXViewPart.java
@@ -27,14 +27,11 @@ import javafx.scene.Scene;
  */
 public class ModelQualityFXViewPart extends AF3FXViewPart implements IModelQualityViewPart {
 
+	// TODO (SB): Is there a reason to make the controller static? (also rename to viewController)
 	/** The FX Controller for this view. */
 	private static final ModelQualityFXController VIEW_CONTROLLER = new ModelQualityFXController();
 
-	/**
-	 * Constructor.
-	 *
-	 * @throws Exception
-	 */
+	/** Constructor. */
 	public ModelQualityFXViewPart() throws Exception {
 		super(VIEW_CONTROLLER, null);
 	}
-- 
GitLab