From 0496ec4e8ee528d280be0dffda49955fa12a1e6d Mon Sep 17 00:00:00 2001
From: blaschke <blaschke@FOR2848.fortiss.fortiss.org>
Date: Mon, 16 Oct 2023 14:57:03 +0200
Subject: [PATCH] Review rework on Quality Plugin
- solving all low hanging fruits from syntax to smaller refactoring
issues
- adding several ToDos (#4331, #4332, #4333) for greater changes due to
missing information or understanding what the expected solution should
exactly look like.
Issue-Ref: 4310
Issue-Url: https://git.fortiss.org/af3/af3/-/issues/4310
Signed-off-by: Konstantin Blaschke <blaschke@fortiss.org>
---
.../fortiss/tooling/ext/quality/ui/.ratings | 2 +-
.../ui/ModelQualityExtractionMenu.java | 12 +-
.../tooling/ext/quality/ui/view/fx/.ratings | 4 +-
.../ui/view/fx/ModelQualityFXController.java | 40 +++--
.../ui/view/fx/ModelQualityFXViewPart.java | 2 +
.../META-INF/MANIFEST.MF | 2 +-
.../html/developer/.ratings | 2 +-
.../html/developer/documentation.html | 142 +++-------------
.../org/fortiss/tooling/ext/quality/.ratings | 6 +-
.../ext/quality/GraphMetricsProvider.java | 151 ++++++++----------
...a => HierarchicElementMetricProvider.java} | 81 +++++-----
...ivator.java => ModelQualityActivator.java} | 17 +-
.../fortiss/tooling/ext/quality/data/.ratings | 8 +-
.../ext/quality/data/DataRootElement.java | 28 +---
...MetricDataManager.java => MetricData.java} | 28 +---
.../tooling/ext/quality/data/MetricKey.java | 90 +++++------
.../ext/quality/data/MetricTreeNode.java | 37 ++---
.../tooling/ext/quality/service/.ratings | 4 +-
.../quality/service/IModelQualityService.java | 16 ++
.../quality/service/ModelQualityService.java | 99 +++++-------
.../tooling/ext/quality/storage/.ratings | 4 +-
.../ext/quality/storage/CSVFileWriter.java | 73 ++++-----
.../storage/ModelQualityStorageManager.java | 24 +--
23 files changed, 341 insertions(+), 531 deletions(-)
rename org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/{HierarchicElementProvider.java => HierarchicElementMetricProvider.java} (54%)
rename org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/{AF3QualityActivator.java => ModelQualityActivator.java} (75%)
rename org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/{MetricDataManager.java => MetricData.java} (64%)
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 4f312f29e..10f0daaec 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 e2926b47f4f30adb160c47aef773853bb5b03fcb RED
+ModelQualityExtractionMenu.java 3bdbf3d37715539648af8ba7feefbec9370c4dc4 YELLOW
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 e2926b47f..3bdbf3d37 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
@@ -27,7 +27,6 @@ import org.eclipse.jface.action.Action;
import org.eclipse.jface.action.ActionContributionItem;
import org.eclipse.jface.action.IContributionItem;
import org.eclipse.jface.resource.ImageDescriptor;
-import org.fortiss.af3.project.model.FileProject;
import org.fortiss.tooling.ext.quality.service.IModelQualityService;
import org.fortiss.tooling.kernel.extension.data.ITopLevelElement;
import org.fortiss.tooling.kernel.service.IPersistencyService;
@@ -48,14 +47,11 @@ 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)) {
+ if(!(selection.eContainer() != null)) {
return emptyList();
}
return asList(new IContributionItem[] {
- new ActionContributionItem(new MetricExtractionAction((FileProject)selection))});
+ new ActionContributionItem(new MetricExtractionAction(selection))});
}
/** {@inheritDoc} */
@@ -73,10 +69,10 @@ public class ModelQualityExtractionMenu implements IContextMenuContributor {
protected class MetricExtractionAction extends Action {
/** {@link FileProject} on which the action will be triggered. */
- private final FileProject fp;
+ private final EObject fp;
/** Constructor. */
- public MetricExtractionAction(FileProject fp) {
+ public MetricExtractionAction(EObject fp) {
super(MENU_NAME, getActionIcon());
this.fp = fp;
}
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 1b3e6d886..c0833d43f 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 GREEN
-ModelQualityFXController.java f4ae42467509d29326bdabe822310083253910dc RED
-ModelQualityFXViewPart.java 4f142d1b04ab9e5f0ce54ff248fe0abf6f141937 RED
+ModelQualityFXController.java 67bb0a067c578ac9a9b30e5973587cc57374a782 YELLOW
+ModelQualityFXViewPart.java a1e82fe68a3fe7d813fa61d5148ffcd1592000f7 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 f4ae42467..67bb0a067 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
@@ -18,6 +18,7 @@ package org.fortiss.tooling.ext.quality.ui.view.fx;
import static javafx.scene.paint.Color.BLUE;
import static javafx.scene.paint.Color.DARKGRAY;
import static javafx.scene.paint.Color.LIGHTGRAY;
+import static java.util.stream.Collectors.toList;
import static org.fortiss.tooling.common.ui.javafx.style.FontStyle.BLACK_VERDANA_10PT;
import static org.fortiss.tooling.common.ui.javafx.style.FontStyle.BLACK_VERDANA_12PT;
import static org.fortiss.tooling.common.ui.javafx.style.FontStyle.BLACK_VERDANA_14PT;
@@ -26,13 +27,14 @@ import static org.fortiss.tooling.common.ui.javafx.style.LineStyle.SOLID_BLACK_1
import static org.fortiss.tooling.kernel.ui.util.SelectionUtils.checkAndPickFirst;
import static org.fortiss.tooling.kernel.utils.EcoreUtils.getFirstParentWithType;
+
import java.text.DecimalFormat;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
-import java.util.stream.Collectors;
+
import org.eclipse.emf.ecore.EObject;
import org.eclipse.jface.viewers.ISelection;
@@ -43,10 +45,10 @@ import org.fortiss.tooling.common.ui.javafx.style.FillStyle;
import org.fortiss.tooling.common.ui.javafx.style.FontStyle;
import org.fortiss.tooling.common.ui.javafx.style.LineStyle;
import org.fortiss.tooling.ext.quality.IMetricUpdateListener;
-import org.fortiss.tooling.ext.quality.data.MetricDataManager;
+import org.fortiss.tooling.ext.quality.data.MetricData;
import org.fortiss.tooling.ext.quality.data.MetricKey;
import org.fortiss.tooling.ext.quality.data.MetricTreeNode;
-import org.fortiss.tooling.ext.quality.service.ModelQualityService;
+import org.fortiss.tooling.ext.quality.service.IModelQualityService;
import org.fortiss.tooling.kernel.model.IProjectRootElement;
import org.fortiss.tooling.spiderchart.control.SpiderChartViewer;
import org.fortiss.tooling.spiderchart.model.DataSeries;
@@ -151,11 +153,7 @@ 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();
+ IModelQualityService mqs = IModelQualityService.getInstance();
EObject selected = checkAndPickFirst(selection, EObject.class);
@@ -172,8 +170,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
updateCharts("Selection does not have a project root element.");
return;
}
-
- var rootDataElement = manager.getDataRootElementMap().get(currentRootElement);
+ var rootDataElement = mqs.getMetricData().getDataRootElementMap().get(currentRootElement);
if(rootDataElement == null) {
bottomText.setText(
@@ -212,9 +209,9 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
private void updateCharts(String message) {
if(lastSelectedElement != null) {
- MetricDataManager manager =
- ModelQualityService.getInstance().getMetricDataManagerInstance();
- MetricTreeNode node = manager.getTreeNodeLookupTable().get(lastSelectedElement);
+ MetricData metricData =
+ IModelQualityService.getInstance().getMetricData();
+ MetricTreeNode node = metricData.getTreeNodeLookupTable().get(lastSelectedElement);
if(node != null) {
@@ -343,10 +340,12 @@ 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());
+ .map(pair -> new Pair<>(pair.getKey(),
+ (pair.getKey().getName() == null) ? new DataSeries("No Name found") : new DataSeries(pair.getKey().getName())
+ ))
+ .collect(toList());
+
// Create SpiderChart and set basic styling
SpiderChart spiderChart = new SpiderChart();
@@ -381,11 +380,9 @@ 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);
+ int i = 0;
+ for(var pair : dataSeriesList) {
spiderChart.addData(pair.getValue());
-
// Assign different colors to different data series
Color color = SPIDER_CHART_OVERLAY_COLORS[i];
LineStyle olive1pt = new LineStyle(color);
@@ -393,6 +390,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
DataSeriesStyle style = new DataSeriesStyle(olive1pt, oliveFill, true, true,
BLACK_VERDANA_10PT, 7, new DecimalFormat("#.#"));
chartStyle.setDataSeriesStyle(pair.getValue(), style);
+ i++;
}
// Configure legend
@@ -419,7 +417,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
// Store IModelQualityService.getInstance() (sic!) in a variable (e.g. "mqs") for
// readability
- var rootDataElement = ModelQualityService.getInstance().getMetricDataManagerInstance()
+ var rootDataElement = IModelQualityService.getInstance().getMetricData()
.getDataRootElementMap().get(updatedElement);
if(rootDataElement != null) {
bottomText.setText("Metrics last updated: " +
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 4f142d1b0..a1e82fe68 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
@@ -28,6 +28,8 @@ 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)
+ // FIXME (KB): Naming convention and static taken from reuse library. So you want me to change the FX controller class to viewController?
+ // Would result in inconsistent use otherwise. still change it and add the view controller as parameter for instancing a View Part?
/** The FX Controller for this view. */
private static final ModelQualityFXController VIEW_CONTROLLER = new ModelQualityFXController();
diff --git a/org.fortiss.tooling.ext.quality/META-INF/MANIFEST.MF b/org.fortiss.tooling.ext.quality/META-INF/MANIFEST.MF
index 31f41cf56..730b39560 100644
--- a/org.fortiss.tooling.ext.quality/META-INF/MANIFEST.MF
+++ b/org.fortiss.tooling.ext.quality/META-INF/MANIFEST.MF
@@ -6,7 +6,7 @@ Bundle-Version: 2.23.0.qualifier
Automatic-Module-Name: org.fortiss.tooling.ext.quality
Bundle-RequiredExecutionEnvironment: JavaSE-11
Bundle-Vendor: fortiss GmbH
-Bundle-Activator: org.fortiss.tooling.ext.quality.AF3QualityActivator
+Bundle-Activator: org.fortiss.tooling.ext.quality.ModelQualityActivator
Require-Bundle: org.eclipse.core.runtime,
org.eclipse.emf.ecore;visibility:=reexport,
org.fortiss.tooling.base;visibility:=reexport,
diff --git a/org.fortiss.tooling.ext.quality/html/developer/.ratings b/org.fortiss.tooling.ext.quality/html/developer/.ratings
index b1f906103..b6a7b0b9a 100644
--- a/org.fortiss.tooling.ext.quality/html/developer/.ratings
+++ b/org.fortiss.tooling.ext.quality/html/developer/.ratings
@@ -1 +1 @@
-documentation.html f102e6b3757eec158de9bf78238e2865d01de2a4 RED
+documentation.html b66c2bfb495047a8e4b680b02231ab2fdb34cec3 YELLOW
diff --git a/org.fortiss.tooling.ext.quality/html/developer/documentation.html b/org.fortiss.tooling.ext.quality/html/developer/documentation.html
index f102e6b37..b66c2bfb4 100644
--- a/org.fortiss.tooling.ext.quality/html/developer/documentation.html
+++ b/org.fortiss.tooling.ext.quality/html/developer/documentation.html
@@ -15,7 +15,6 @@
<table border="1">
<thead>
<tr>
- <th>Number</th>
<th>Provider</th>
<th>Qualified Name</th>
<th>Metric</th>
@@ -25,95 +24,83 @@
</thead>
<tbody>
<tr>
- <td>1</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>UNIQUE_ID</td>
<td>Integer</td>
<td>This is an unique identifier for each element as specified in org.fortiss.tooling.kernel.model.IIdLabeled.</td>
</tr>
<tr>
- <td>2</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_CONNECTORS</td>
<td>Integer</td>
<td>Number of connectors in the respective element.</td>
</tr>
<tr>
- <td>3</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_CONTAINED_ELEMENTS</td>
<td>Integer</td>
<td>Number of entries in the containedElements list.</td>
</tr>
<tr>
- <td>4</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_CONNECTIONS</td>
<td>Integer</td>
<td>Number of connections in this element. List of aggregated connection model elements. Usually aggregates all connections of its direct sub-structure.</td>
</tr>
<tr>
- <td>5</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_TOTAL_CONNECTORS</td>
<td>Integer</td>
<td>Total number of connectors in this element and all contained elements.</td>
</tr>
<tr>
- <td>6</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_TOTAL_ENTRY_CONNECTORS</td>
<td>Integer</td>
<td>Sum of all connectors which implement org.fortiss.tooling.base.model.base.EntryConnectorBase.</td>
</tr>
<tr>
- <td>7</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_TOTAL_EXIT_CONNECTORS</td>
<td>Integer</td>
<td>Sum of all connectors which implement org.fortiss.tooling.base.model.base.ExitConnectorBase. Usually, the sum over this and NUMBER_OF_TOTAL_ENTRY_CONNECTORS should equal NUMBER_OF_TOTAL_CONNECTORS.</td>
</tr>
<tr>
- <td>8</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_TOTAL_ELEMENTS</td>
<td>Integer</td>
<td>Total number of elements contained in this element. Includes the element for which this metric is recorded.</td>
</tr>
<tr>
- <td>9</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_TOTAL_COMMENTABLE_ELEMENTS</td>
<td>Integer</td>
<td>Total number of elements implementing org.fortiss.tooling.kernel.model.INamedCommentedElement in this element. Includes the element for which this metric is recorded. Never exceeds NUMBER_OF_TOTAL_ELEMENTS.</td>
</tr>
<tr>
- <td>10</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_TOTAL_COMMENTED_ELEMENTS</td>
<td>Integer</td>
<td>Total number of elements which are commentable and do not have null or an empty string as a comment.</td>
</tr>
<tr>
- <td>11</td>
- <td>HierarchicElementProvider</td>
- <td>org.fortiss.tooling.ext.quality.HierarchicElementProvider</td>
+ <td>HierarchicElementMetricProvider</td>
+ <td>org.fortiss.tooling.ext.quality.HierarchicElementMetricProvider</td>
<td>NUMBER_OF_TOTAL_LEAF_ELEMENTS</td>
<td>Integer</td>
<td>Total number of elements contained in this element which do not contain elements. Includes self.</td>
</tr>
<tr>
- <td>12</td>
<td>GraphMetricsProvider</td>
<td>org.fortiss.tooling.ext.quality.GraphMetricsProvider</td>
<td>BETWEENESS_CENTRALITY</td>
@@ -121,7 +108,6 @@
<td>Value of the betweenness centrality of this element embedded in a graph of consisting of all elements which are contained in the element which contains this element, as well as all neighbors of these elements.</td>
</tr>
<tr>
- <td>13</td>
<td>GraphMetricsProvider</td>
<td>org.fortiss.tooling.ext.quality.GraphMetricsProvider</td>
<td>BETWEENESS_CENTRALITY_RECURSIVELY</td>
@@ -129,7 +115,6 @@
<td>Same as above, but all connections are resolved to leaf elements, i.e. elements which do not contain other elements.</td>
</tr>
<tr>
- <td>14</td>
<td>GraphMetricsProvider</td>
<td>org.fortiss.tooling.ext.quality.GraphMetricsProvider</td>
<td>CLUSTERING_COEFFICIENT</td>
@@ -137,87 +122,6 @@
<td>Clustering coefficient of this element. Ignores connection direction and resolves all neighbors to leaf elements.</td>
</tr>
<tr>
- <td>15</td>
- <td>ComponentMetricProvider</td>
- <td>TODO (SB): Documentation of providers should be distributed to respective plugins. org.fortiss.af3.component.quality.ComponentMetricProvider</td>
- <td>NUMBER_OF_TOTAL_CODE_CONSTANTS</td>
- <td>Integer</td>
- <td>Total number of constants in code specifications in this element and all contained elements.</td>
- </tr>
- <tr>
- <td>16</td>
- <td>ComponentMetricProvider</td>
- <td>org.fortiss.af3.component.quality.ComponentMetricProvider</td>
- <td>NUMBER_OF_TOTAL_CODE_CYCLOMATIC_COMPLEXITY</td>
- <td>Integer</td>
- <td>The total cyclomatic complexity of all code specifications in this element and all contained elements.</td>
- </tr>
- <tr>
- <td>17</td>
- <td>ComponentMetricProvider</td>
- <td>org.fortiss.af3.component.quality.ComponentMetricProvider</td>
- <td>NUMBER_OF_TOTAL_CODE_LINES</td>
- <td>Integer</td>
- <td>Total number of lines of code in all code specifications in this element and all contained elements.</td>
- </tr>
- <tr>
- <td>18</td>
- <td>ComponentMetricProvider</td>
- <td>org.fortiss.af3.component.quality.ComponentMetricProvider</td>
- <td>NUMBER_OF_TOTAL_CODE_COMMENTS</td>
- <td>Integer</td>
- <td>Total number of comment blocks in all code specifications in this element and all contained elements.</td>
- </tr>
- <tr>
- <td>19</td>
- <td>DataDictonaryProvider</td>
- <td>org.fortiss.af3.component.quality.DataDictonaryProvider</td>
- <td>-</td>
- <td>-</td>
- <td>Provides Metrics also collected by the HierarchicElementProvider, but for the DataDictionary.</td>
- </tr>
- <tr>
- <td>20</td>
- <td>SafetyMetricProvider</td>
- <td>org.fortiss.af3.safety.quality.SafetyMetricProvider</td>
- <td>SAFETY_LEVEL</td>
- <td>String</td>
- <td>String representation of the assigned safety level of this element.</td>
- </tr>
- <tr>
- <td>21</td>
- <td>SafetyMetricProvider</td>
- <td>org.fortiss.af3.safety.quality.SafetyMetricProvider</td>
- <td>SAFETY_LEVEL</td>
- <td>String</td>
- <td>String representation of the assigned safety level of this element.</td>
- </tr>
- <tr>
- <td>22</td>
- <td>SafetyMetricProvider</td>
- <td>org.fortiss.af3.safety.quality.SafetyMetricProvider</td>
- <td>SAFETY_LEVEL</td>
- <td>String</td>
- <td>String representation of the assigned safety level of this element.</td>
- </tr>
- <tr>
- <td>23</td>
- <td>AllocationTableProvider</td>
- <td>org.fortiss.af3.task.quality.AllocationTableProvider</td>
- <td>TASK_ALLOCATED_COMPONENTS</td>
- <td>Integer</td>
- <td>Number of allocated Components on this Task. This metric can only be non-null for org.fortiss.af3.task.model.Task Elements.</td>
- </tr>
- <tr>
- <td>24</td>
- <td>AllocationTableProvider</td>
- <td>org.fortiss.af3.task.quality.AllocationTableProvider</td>
- <td>HARDWARE_ALLOCATED_TASKS</td>
- <td>Integer</td>
- <td>Number of allocated Tasks on this ExecutionUnit. This metric can only be non-null for org.fortiss.af3.platform.model.ExecutionUnit Elements.</td>
- </tr>
- <tr>
- <td>25</td>
<td>ModelQualityService</td>
<td>org.fortiss.tooling.ext.quality.service.ModelQualityService</td>
<td>NESTING_LEVEL</td>
@@ -225,7 +129,6 @@
<td>The depth of this element. Defined by the number of elements which have to be traversed to reach this element from the root.</td>
</tr>
<tr>
- <td>26</td>
<td>ModelQualityService</td>
<td>org.fortiss.tooling.ext.quality.service.ModelQualityService</td>
<td>CONSTRAINT_VIOLATIONS_ERROR</td>
@@ -233,7 +136,6 @@
<td>Number of constraint violations of the severity error which have this element as source.</td>
</tr>
<tr>
- <td>27</td>
<td>ModelQualityService</td>
<td>org.fortiss.tooling.ext.quality.service.ModelQualityService</td>
<td>CONSTRAINT_VIOLATIONS_WARNING</td>
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 a0dfdcdd3..f93b691d3 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,5 +1,5 @@
-AF3QualityActivator.java 9f2f29b8d55c033e1df16320b0b8a1b6f0446f60 RED
-GraphMetricsProvider.java 0d760fe19417a080016ebc4bfcc4c2753bde1e59 RED
-HierarchicElementProvider.java cbe405204b08931fd5d219570d0ffb2ab49f27b7 RED
+GraphMetricsProvider.java d18cff37dcb103a447cead64e1c2b00866996b3c YELLOW
+HierarchicElementMetricProvider.java e76e846d76c6ba8782a20d9c9f9c6d57eaaa416e YELLOW
IMetricProvider.java 99fc8993b0e65b2f8757978eeb0481d912f5608c GREEN
IMetricUpdateListener.java c24dc7c0f282623bbf1eefac1fbbb6752c97ddf0 GREEN
+ModelQualityActivator.java 6d5f0794aa48670cf45fb405bd7641bce6c1fec4 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 0d760fe19..d18cff37d 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
@@ -25,15 +25,16 @@ import java.util.Queue;
import java.util.Set;
import java.util.Stack;
+import static org.fortiss.tooling.kernel.utils.EcoreUtils.pickInstanceOf;
+
import org.eclipse.emf.common.util.EList;
import org.fortiss.tooling.base.model.base.EntryConnectorBase;
import org.fortiss.tooling.base.model.base.ExitConnectorBase;
import org.fortiss.tooling.base.model.element.IConnection;
import org.fortiss.tooling.base.model.element.IConnector;
import org.fortiss.tooling.base.model.element.IHierarchicElement;
-import org.fortiss.tooling.ext.quality.data.MetricDataManager;
+import org.fortiss.tooling.ext.quality.data.MetricData;
import org.fortiss.tooling.ext.quality.data.MetricKey;
-import org.fortiss.tooling.kernel.utils.EcoreUtils;
/**
* Supplies methods to compute certain graph metrics.
@@ -41,9 +42,7 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
* @author groh
*/
-// TODO (SB): shouldn't this implement the interface IMetricProvider? If it is not possible to
-// factorize this now, but the suggestions makes sense, create a ticket and reference it here
-// using TODO (#xyz): blah). Otherwise, consider renaming the class to avoid the confusion.
+// TODO (#4332) Restructure GraphMetricsProvider in quality Plugin
public class GraphMetricsProvider {
/**
@@ -68,17 +67,15 @@ public class GraphMetricsProvider {
graphNodes.addAll(scopeElement.getContainedElements());
}
- // TODO (SB): Static import
// Collect all elements to which currentElement has a outgoing connection
- for(ExitConnectorBase exitConnector : EcoreUtils.pickInstanceOf(ExitConnectorBase.class,
+ for(ExitConnectorBase exitConnector : pickInstanceOf(ExitConnectorBase.class,
scopeElement.getConnectors())) {
for(IConnection exitConnection : exitConnector.getOutgoing()) {
graphNodes.add(exitConnection.getTarget().getOwner());
}
}
- // TODO (SB): Static import
// Collect all elements to which currentElement has a incoming connection
- for(EntryConnectorBase entryConnector : EcoreUtils.pickInstanceOf(EntryConnectorBase.class,
+ for(EntryConnectorBase entryConnector : pickInstanceOf(EntryConnectorBase.class,
scopeElement.getConnectors())) {
for(IConnection entryConnection : entryConnector.getIncoming()) {
graphNodes.add(entryConnection.getSource().getOwner());
@@ -161,24 +158,22 @@ public class GraphMetricsProvider {
private static Set<IHierarchicElement> getUndirectedNeighbors(IHierarchicElement element) {
Set<IHierarchicElement> neighbors = new HashSet<>();
- // TODO (SB): Static import
- for(ExitConnectorBase exitConnectors : EcoreUtils.pickInstanceOf(ExitConnectorBase.class,
+ for(ExitConnectorBase exitConnectors : pickInstanceOf(ExitConnectorBase.class,
element.getConnectors())) {
recursivlyFollowOutgoingConnection(exitConnectors, neighbors);
}
- // TODO (SB): Static import
- for(EntryConnectorBase entryConnectors : EcoreUtils.pickInstanceOf(EntryConnectorBase.class,
+ for(EntryConnectorBase entryConnectors : pickInstanceOf(EntryConnectorBase.class,
element.getConnectors())) {
recursivlyFollowIncomingConnection(entryConnectors, neighbors);
}
return neighbors;
}
- // TODO (SB): Pleae provide a link (DOI to paper, Wikipedia, ...) to a definition of the
- // clustering coefficient? The algorithm is not obvious...
/**
* Calculates the clustering coefficient based on the neighborhood.
- *
+ * https://en.wikipedia.org/wiki/Clustering_coefficient
+ * Clustering coefficient C for a node V is counting the triangles divided by the "number of neighbors of
+ * V" times "number of neighbors of V - 1" which results in the potential number of maximum links between the nodes
* @param element
* of which the coefficient shall be calculated
* @return value of the coefficient
@@ -186,19 +181,15 @@ public class GraphMetricsProvider {
public static double calculateClusteringCoefficent(IHierarchicElement element) {
Set<IHierarchicElement> neighbors = getUndirectedNeighbors(element);
-
int totalEdges = neighbors.size();
if(totalEdges < 2) {
return 0.0; // If there are fewer than 2 neighbors, the coefficient is undefined (or 0).
}
-
// Iterate over neighbors and get their neighbors and check if they are also our neighbor in
// which case we have a triangle
int triangles = 0;
for(IHierarchicElement neighbor : neighbors) {
-
Set<IHierarchicElement> neighborNeighbors = getUndirectedNeighbors(neighbor);
-
for(IHierarchicElement secondNeighbor : neighbors) {
if(secondNeighbor != neighbor && neighborNeighbors.contains(secondNeighbor)) {
triangles++;
@@ -209,92 +200,90 @@ public class GraphMetricsProvider {
return clusteringCoefficient;
}
- // TODO (SB): Please provide a link (DOI to paper, Wikipedia, ...) to a definition of the
- // betweenness centrality. The algorithm is not obvious...
/**
* Calculates and saves the betweenness centrality of all elements contained inside the provided
- * scopeElement.
+ * scopeElement. Described for example in https://en.wikipedia.org/wiki/Betweenness_centrality
*
* @param scopeElement
* the scope of this calculation
- * @param manager
+ * @param metricData
* location to save the metrics
* @param recursively
* if true, all calculation will be carried out on leaf elements
- * TODO (SB):, otherwise.... <not obvious :-) >
+ * if false, it is only computing the centrality with the children
+ * which means the inner circle.
*/
public static void calculateBetweennessCentrality(IHierarchicElement scopeElement,
- MetricDataManager manager, boolean recursively) {
+ MetricData metricData, boolean recursively) {
- // TODO (SB): Use inverse logic and return to decrease nesting depth
- // if (foo) { return; } bar();
// Abort if no elements are found
- if(!scopeElement.getContainedElements().isEmpty()) {
+ if(scopeElement.getContainedElements().isEmpty()) {
+ return;
+ }
+
+ Set<IHierarchicElement> graphNodes = getLocalGraphView(scopeElement, recursively);
+ // Initialize the betweenness values for all nodes to zero
+ Map<IHierarchicElement, Double> betweenness = new HashMap<>();
+ for(IHierarchicElement elementNode : graphNodes) {
+ betweenness.put(elementNode, 0.0);
+ }
- Set<IHierarchicElement> graphNodes = getLocalGraphView(scopeElement, recursively);
- // Initialize the betweenness values for all nodes to zero
- Map<IHierarchicElement, Double> betweenness = new HashMap<>();
- for(IHierarchicElement elementNode : graphNodes) {
- betweenness.put(elementNode, 0.0);
- }
+ // Iterate over all collected nodes
+ for(IHierarchicElement elementNode : graphNodes) {
- // Iterate over all collected nodes
- for(IHierarchicElement elementNode : graphNodes) {
+ // Initialize all variables with standard values
+ Stack<IHierarchicElement> stack = new Stack<>();
+ Map<IHierarchicElement, List<IHierarchicElement>> predecessors = new HashMap<>();
+ Map<IHierarchicElement, Integer> distance = new HashMap<>();
+ Map<IHierarchicElement, Double> dependency = new HashMap<>();
+ Queue<IHierarchicElement> queue = new LinkedList<>();
- // Initialize all variables
- Stack<IHierarchicElement> stack = new Stack<>();
- 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) {
+ predecessors.put(v, new ArrayList<>());
+ distance.put(v, -1);
+ dependency.put(v, 0.0);
+ }
+ dependency.put(elementNode, 1.0);
+ distance.put(elementNode, 0);
+ queue.add(elementNode);
- for(IHierarchicElement v : graphNodes) {
- predecessors.put(v, new ArrayList<>());
- distance.put(v, -1);
- dependency.put(v, 0.0);
- }
- dependency.put(elementNode, 1.0);
- distance.put(elementNode, 0);
- queue.add(elementNode);
+ processCentrality(queue, stack, predecessors, distance, dependency, graphNodes,
+ elementNode, recursively);
- processCentrality(queue, stack, predecessors, distance, dependency, graphNodes,
- elementNode, recursively);
+ // Initialize delta
+ Map<IHierarchicElement, Double> delta = new HashMap<>();
+ for(IHierarchicElement v : graphNodes) {
+ delta.put(v, 0.0);
+ }
- // Initialize delta
- Map<IHierarchicElement, Double> delta = new HashMap<>();
- for(IHierarchicElement v : graphNodes) {
- delta.put(v, 0.0);
+ while(!stack.isEmpty()) {
+ IHierarchicElement w = stack.pop();
+ 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);
}
-
- while(!stack.isEmpty()) {
- IHierarchicElement w = stack.pop();
- 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)) {
- betweenness.put(w, betweenness.get(w) + delta.get(w));
- }
+ if(!w.equals(elementNode)) {
+ betweenness.put(w, betweenness.get(w) + delta.get(w));
}
}
- // Save metric
- saveMetric(scopeElement, manager, recursively, betweenness);
}
+ // Save metric
+ saveMetric(scopeElement, metricData, recursively, betweenness);
}
/** Saving the metrics for each contained element of an {@link IHierarchicElement}. */
- private static void saveMetric(IHierarchicElement scopeElement, MetricDataManager manager,
+ private static void saveMetric(IHierarchicElement scopeElement, MetricData metricData,
boolean recursively, Map<IHierarchicElement, Double> betweenness) {
MetricKey key = recursively ? MetricKey.BETWEENESS_CENTRALITY_RECURSIVELY
: MetricKey.BETWEENESS_CENTRALITY;
for(IHierarchicElement child : scopeElement.getContainedElements()) {
- manager.getTreeNodeLookupTable().get(child).getStoredDoubles().put(key,
+ metricData.getTreeNodeLookupTable().get(child).getStoredDoubles().put(key,
betweenness.get(child));
}
}
/**
- * Performs intermediate calculations for the betweenness centrality algorithm
+ * Performs intermediate calculations for the betweenness centrality algorithm.
*
* @param queue
* of nodes for Breadth-First Search.
@@ -327,8 +316,7 @@ public class GraphMetricsProvider {
stack.push(v);
// Iterate over all connectors going out from the element
- for(ExitConnectorBase exitConnectors : EcoreUtils
- .pickInstanceOf(ExitConnectorBase.class, v.getConnectors())) {
+ for(ExitConnectorBase exitConnectors : pickInstanceOf(ExitConnectorBase.class, v.getConnectors())) {
// Iterate over all connections
for(IConnection outgoingConnection : exitConnectors.getOutgoing()) {
@@ -359,16 +347,13 @@ public class GraphMetricsProvider {
// Check if element is in node list, to avoid creating data for
// nodes outside of our graph
if(graphNodes.contains(targetElement)) {
- // TODO (SB): Introduce local variables with good names for
- // distance.get(targetElement) and distance.get(v)
- if(distance.get(targetElement) < 0) {
+ var targetDistance = distance.get(targetElement);
+ var vDistance = distance.get(v);
+ // first start of distance calculation from v to the target Element
+ if(targetDistance < 0) {
queue.add(targetElement);
- distance.put(targetElement, distance.get(v) + 1);
- }
- // TODO (SB): Just for safety-checking: Is it intended, there there is
- // no "else", i.e. that both conditions could be true and both branches
- // should be entered?
- if(distance.get(targetElement) == distance.get(v) + 1) {
+ distance.put(targetElement, vDistance + 1);
+ }else if(targetDistance == vDistance + 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/HierarchicElementProvider.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementMetricProvider.java
similarity index 54%
rename from org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementProvider.java
rename to org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementMetricProvider.java
index cbe405204..e76e846d7 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementProvider.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementMetricProvider.java
@@ -15,74 +15,79 @@
+--------------------------------------------------------------------------*/
package org.fortiss.tooling.ext.quality;
+import static org.fortiss.tooling.kernel.utils.EcoreUtils.pickInstanceOf;
+
import org.fortiss.tooling.base.model.base.EntryConnectorBase;
import org.fortiss.tooling.base.model.base.ExitConnectorBase;
+
import org.fortiss.tooling.base.model.element.IHierarchicElement;
import org.fortiss.tooling.ext.quality.data.MetricKey;
import org.fortiss.tooling.ext.quality.data.MetricTreeNode;
import org.fortiss.tooling.kernel.model.IIdLabeled;
import org.fortiss.tooling.kernel.model.INamedCommentedElement;
-import org.fortiss.tooling.kernel.utils.EcoreUtils;
/**
* {@link IMetricProvider} to collect various metrics from an {@link IHierarchicElement}.
*
* @author groh
*/
-// TODO (SB): Rename to HierarchicElementMetricProvider
-public class HierarchicElementProvider implements IMetricProvider<IHierarchicElement> {
+public class HierarchicElementMetricProvider implements IMetricProvider<IHierarchicElement> {
/** {@inheritDoc} */
@Override
public void collectMetrics(MetricTreeNode node, IHierarchicElement currentElement) {
- // TODO (SB): Use better variable name, e.g. intMetrics. See also comment in MetricTreeNode
- var integers = node.getStoredIntegers();
+ var intMetrics = node.getStoredIntegers();
// Only collect these metrics if the name is null, in which case they have not been
// collected before
// This is used in some cases to combine metrics from two different IHierarchicElement
if(node.getName() == null) {
// Metrics are always set to default value, as these metric are combined later
- // TODO (SB): The ternary operators here very hard to read, and later on you use an if
- // anyways.
- // Suggestion: if (currentElement instanceof INamedCommentedElement) { bla} else {
- // blub}.
- boolean isElementCommentable = currentElement instanceof INamedCommentedElement;
- integers.put(MetricKey.NUMBER_OF_TOTAL_COMMENTABLE_ELEMENTS,
- isElementCommentable ? 1 : 0);
- String comment = isElementCommentable
- ? ((INamedCommentedElement)currentElement).getComment() : null;
- integers.put(MetricKey.NUMBER_OF_TOTAL_COMMENTED_ELEMENTS,
- comment != null && comment != "" ? 1 : 0);
- if(isElementCommentable) {
+ String comment;
+ if(currentElement instanceof INamedCommentedElement) {
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_COMMENTABLE_ELEMENTS, 1);
+ comment =((INamedCommentedElement)currentElement).getComment();
+ if(comment != "") {
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_COMMENTED_ELEMENTS, 1);
+ }
node.setName(((INamedCommentedElement)currentElement).getName());
- } else {
- node.setName("Unnamable Element");
+ }else {
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_COMMENTABLE_ELEMENTS, 0);
+ comment = null;
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_COMMENTED_ELEMENTS, 0);
+ node.setName("No Name existant");
}
-
- integers.put(MetricKey.UNIQUE_ID, (currentElement instanceof IIdLabeled)
- ? ((IIdLabeled)currentElement).getId() : -1);
+
+ if(currentElement instanceof IIdLabeled) {
+ intMetrics.put(MetricKey.UNIQUE_ID,((IIdLabeled)currentElement).getId());
+ }else {
+ intMetrics.put(MetricKey.UNIQUE_ID, -1);
+ }
}
+ // connector metrics
var connectors = currentElement.getConnectors();
- integers.put(MetricKey.NUMBER_OF_CONNECTORS, connectors.size());
-
- integers.put(MetricKey.NUMBER_OF_CONTAINED_ELEMENTS,
+ intMetrics.put(MetricKey.NUMBER_OF_CONNECTORS, connectors.size());
+ intMetrics.put(MetricKey.NUMBER_OF_CONTAINED_ELEMENTS,
currentElement.getContainedElements().size());
- integers.put(MetricKey.NUMBER_OF_CONNECTIONS, currentElement.getConnections().size());
-
+ intMetrics.put(MetricKey.NUMBER_OF_CONNECTIONS, currentElement.getConnections().size());
+
// depth metrics
- integers.put(MetricKey.NUMBER_OF_TOTAL_CONNECTORS, connectors.size());
- // TODO (SB): Static import (CTRL+SHIFT+M). (2x)
- var entry_connectors = EcoreUtils.pickInstanceOf(EntryConnectorBase.class, connectors);
- var exit_connectors = EcoreUtils.pickInstanceOf(ExitConnectorBase.class, connectors);
- integers.put(MetricKey.NUMBER_OF_TOTAL_ENTRY_CONNECTORS, entry_connectors.size());
- integers.put(MetricKey.NUMBER_OF_TOTAL_EXIT_CONNECTORS, exit_connectors.size());
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_CONNECTORS, connectors.size());
+
+ var entry_connectors = pickInstanceOf(EntryConnectorBase.class, connectors);
+ var exit_connectors = pickInstanceOf(ExitConnectorBase.class, connectors);
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_ENTRY_CONNECTORS, entry_connectors.size());
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_EXIT_CONNECTORS, exit_connectors.size());
+
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_ELEMENTS, 1);
- integers.put(MetricKey.NUMBER_OF_TOTAL_ELEMENTS, 1);
+ if (currentElement.getContainedElements().isEmpty()) {
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_LEAF_ELEMENTS, 0);
+ }else {
+ intMetrics.put(MetricKey.NUMBER_OF_TOTAL_LEAF_ELEMENTS, 1);
+ }
- // TODO (SB): size() == 0 --> isEmpty()
- integers.put(MetricKey.NUMBER_OF_TOTAL_LEAF_ELEMENTS,
- currentElement.getContainedElements().size() == 0 ? 1 : 0);
- }
+ //graph metrics
+ }
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/AF3QualityActivator.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/ModelQualityActivator.java
similarity index 75%
rename from org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/AF3QualityActivator.java
rename to org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/ModelQualityActivator.java
index 9f2f29b8d..6d5f0794a 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/AF3QualityActivator.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/ModelQualityActivator.java
@@ -19,21 +19,19 @@ package org.fortiss.tooling.ext.quality;
import org.eclipse.core.runtime.Plugin;
import org.fortiss.tooling.base.model.element.IHierarchicElement;
import org.fortiss.tooling.ext.quality.service.IModelQualityService;
-import org.fortiss.tooling.ext.quality.service.ModelQualityService;
import org.osgi.framework.BundleContext;
-// TODO (SB): Rename class to QualityActivator or ModelQualityActivator. This is the kernel, not AF3
/**
* The activator class controls the plug-in life cycle.
*
* @author blaschke
*/
-public class AF3QualityActivator extends Plugin {
+public class ModelQualityActivator extends Plugin {
/** The plug-in ID. */
- public static final String PLUGIN_ID = AF3QualityActivator.class.getPackage().getName(); // $NON-NLS-1$
+ public static final String PLUGIN_ID = ModelQualityActivator.class.getPackage().getName(); // $NON-NLS-1$
/** The shared instance. */
- private static AF3QualityActivator plugin;
+ private static ModelQualityActivator plugin;
/** {@inheritDoc} */
@Override
@@ -41,10 +39,9 @@ public class AF3QualityActivator extends Plugin {
super.start(context);
plugin = this;
System.out.println("[Plugin] " + PLUGIN_ID + " started.");
-
- // TODO: Use only IModelQualityService. Store it in a variable with a brief name, e.g., mqs
- ModelQualityService.getInstance().startService();
- IModelQualityService.getInstance().registerMetricProvider(new HierarchicElementProvider(),
+ IModelQualityService mqs = IModelQualityService.getInstance();
+ mqs.startService();
+ mqs.registerMetricProvider(new HierarchicElementMetricProvider(),
IHierarchicElement.class);
}
@@ -56,7 +53,7 @@ public class AF3QualityActivator extends Plugin {
}
/** Returns the shared instance. */
- public static AF3QualityActivator getDefault() {
+ public static ModelQualityActivator getDefault() {
return plugin;
}
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/.ratings b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/.ratings
index fbdbd5aef..03a1517ed 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/.ratings
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/.ratings
@@ -1,4 +1,4 @@
-DataRootElement.java ab41ea9b52b095be3c03a85a6d1a3036b3026062 RED
-MetricDataManager.java dd2a265e4ac2738b5b68048111597902a763d79f RED
-MetricKey.java ac73424d340e52df84d6ad202d43d0cef9aeba78 RED
-MetricTreeNode.java 0e289fb25da429e11a931dec23d34204d7f12dd2 RED
+DataRootElement.java e598763b5199c4810da191623a07a9edeb154a84 YELLOW
+MetricData.java 03fd1cf0477a42f1f50c899c180035498d21faa3 YELLOW
+MetricKey.java 81ba35f8537c4cb9c0f487a2bfafdb65f9d6c3c0 YELLOW
+MetricTreeNode.java 8eb3c6761a3af482215d48e1bada53415d4ba8e4 YELLOW
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/DataRootElement.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/DataRootElement.java
index ab41ea9b5..e598763b5 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/DataRootElement.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/DataRootElement.java
@@ -27,8 +27,7 @@ import org.fortiss.tooling.kernel.extension.data.ITopLevelElement;
* @author groh
*/
public class DataRootElement {
- // TODO (SB): Comments start with a capital letter, end with a dot, and are free of typos.
- /** the last time a metric tree was refreshed */
+ /** The last time a metric tree was refreshed. */
private LocalDateTime lastRefresh;
/** the last time the metric tree was refreshed */
@@ -37,16 +36,7 @@ public class DataRootElement {
/** Root {@link MetricTreeNode} for the metric tree containg all data */
private MetricTreeNode rootNode;
- /**
- * Constructor: Creates a new instance of the {@link DataRootElement}.
- *
- * @param lastRefresh
- * the last time the rootNode was refreshed
- * @param topLevelElement
- * the {@link ITopLevelElement} of the project
- * @param rootNode
- * the root node of the metric data tree
- */
+ /**Constructor. */
public DataRootElement(LocalDateTime lastRefresh, ITopLevelElement topLevelElement,
MetricTreeNode rootNode) {
super();
@@ -55,34 +45,32 @@ public class DataRootElement {
this.rootNode = rootNode;
}
- /** Returns {@link #lastRefresh}. */
+ /** Getter for {@link #lastRefresh}. */
public LocalDateTime getLastRefresh() {
return lastRefresh;
}
- // TODO (SB): Link the field as I showed you above (5x). If you add an "#" in front of the field
- // name, auto-complete works.
- /** Sets lastRefresh. */
+ /** Setter for {@link #lastRefresh}. */
public void setLastRefresh(LocalDateTime lastRefresh) {
this.lastRefresh = lastRefresh;
}
- /** Returns topLevelElement. */
+ /** Getter for {@link #topLevelElement}. */
public ITopLevelElement getTopLevelElement() {
return topLevelElement;
}
- /** Sets topLevelElement. */
+ /** Setter for {@link #topLevelElement}. */
public void setTopLevelElement(ITopLevelElement topLevelElement) {
this.topLevelElement = topLevelElement;
}
- /** Returns rootNode. */
+ /** Getter for {@link #MetricTreeNode}. */
public MetricTreeNode getRootNode() {
return rootNode;
}
- /** Sets rootNode. */
+ /** Setter for {@link #MetricTreeNode}. */
public void setRootNode(MetricTreeNode metricTreeNode) {
this.rootNode = metricTreeNode;
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricDataManager.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricData.java
similarity index 64%
rename from org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricDataManager.java
rename to org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricData.java
index dd2a265e4..03fd1cf04 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricDataManager.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricData.java
@@ -21,45 +21,31 @@ import java.util.Map;
import org.eclipse.emf.ecore.EObject;
import org.fortiss.tooling.kernel.model.IProjectRootElement;
-// TODO (SB): Rename class name to something like "MetricData". It is not a manager, but a data container.
-// TODO (SB): Remove af3 from the comment. We are in the kernel here!
/**
- * Class containing all computed metrics for the current af3 instance.
+ * Class storing the metrics computed for the all suitable elemnets.
*
* @author groh
*/
-public class MetricDataManager {
+public class MetricData {
- // TODO (SB): Comments start with a capital letter and end with a dot.
- /** a map to lookup the corresponding tree node for any {@link EObject} */
+ /** A map to lookup the corresponding tree node for any {@link EObject}. */
private Map<EObject, MetricTreeNode> treeNodeLookupTable;
- /** a map to lookup the corresponding root tree node for a project */
+ /** A map to lookup the corresponding root tree node for a project. */
private Map<IProjectRootElement, DataRootElement> dataRootElementMap;
- // TODO (SB): Constructors are commented just with "Constructor." (one line).
- /**
- * Constructor: Creates a new instance of the MetricDataManager.
- */
- public MetricDataManager() {
+ /** Constructor.*/
+ public MetricData() {
dataRootElementMap = new HashMap<>();
treeNodeLookupTable = new HashMap<>();
}
- // TODO (SB): You can just use the following comment (avoids inconsistencies).
/** Getter for {@link #treeNodeLookupTable}. */
- /**
- * @return the lookup mapping between {@link EObject} and a {@link MetricTreeNode}.
- */
public Map<EObject, MetricTreeNode> getTreeNodeLookupTable() {
return treeNodeLookupTable;
}
- // TODO (SB): As above
- /**
- * @return the map between {@link IProjectRootElement} and the corresponding root
- * {@link DataRootElement}.
- */
+ /** Getter for {@link #DataRootElementMap}. */
public Map<IProjectRootElement, DataRootElement> getDataRootElementMap() {
return dataRootElementMap;
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricKey.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricKey.java
index ac73424d3..81ba35f85 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricKey.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricKey.java
@@ -20,85 +20,68 @@ import java.util.List;
/**
* Enum class representing a key which can be used to store a respective metric.
- *
+ * Metrics provided by the HierarchicElementProvider.
* @author groh
*/
public enum MetricKey {
- //
- // Metrics provided by the HierarchicElementProvider
- // TODO (SB): All comments should end with a dot.
- //
/** Unique ID of the element */
UNIQUE_ID(false),
- /** Key for the metric counting ports */
+ /** Key for the metric counting ports. */
NUMBER_OF_CONNECTORS(false),
- /** Key for the metric counting elements */
+ /** Key for the metric counting elements. */
NUMBER_OF_CONTAINED_ELEMENTS(false),
- /** Key for the metric counting channels */
+ /** Key for the metric counting channels. */
NUMBER_OF_CONNECTIONS(false),
- // TODO (SB): This adds knowledge about the AF3 metamodel to the kernel. Create a ticket and a
- // TODO(#xyz) for a cleaner solution.
- /** Safety level of the element */
+ // TODO (#4333):
+ /** Safety level of the element. */
SAFETY_LEVEL(false, true),
- /** How deeply nested the corresponding element is */
+ /** How deeply nested the corresponding element is. */
NESTING_LEVEL(false),
- /** The betweenness centrality of the corresponding element */
+ /** The betweenness centrality of the corresponding element. */
BETWEENESS_CENTRALITY(false, 0.0),
/**
* The betweenness centrality of the corresponding element considering other elements
- * recursively
+ * recursively.
*/
BETWEENESS_CENTRALITY_RECURSIVELY(false, 0.0),
/** The clustering coefficient of the element considering its neighbors. */
CLUSTERING_COEFFICIENT(false, 0.0),
- /** How many constraints this element violates with severity error */
+ /** How many constraints this element violates with severity error. */
CONSTRAINT_VIOLATIONS_ERROR(false),
- /** How many constraints this element violates with severity warning */
+ /** How many constraints this element violates with severity warning. */
CONSTRAINT_VIOLATIONS_WARNING(false),
-
- // TODO (SB): This adds knowledge about the AF3 metamodel to the kernel. Create a ticket and a
- // TODO(#xyz) for a cleaner solution. (see above)
- /** Number of allocated components to this task */
+ // TODO(#4333) Remove knowledge about af3 from the kernel.
+ /** Number of allocated components to this task. */
TASK_ALLOCATED_COMPONENTS(false),
- // TODO (SB): This adds knowledge about the AF3 metamodel to the kernel. Create a ticket and a
- // TODO(#xyz) for a cleaner solution. (see above)
- /** How many constraints this element violates with severity warning */
+ // TODO (#4333): Remove knowledge about af3 from the kernel.
+ /** How many constraints this element violates with severity warning. */
EXECUTION_UNIT_ALLOCATED_TASKS(false),
- //
// Metrics which are collected over all children nodes
- //
- /** Key for the metric counting the total amount of ports contained */
+ /** Key for the metric counting the total amount of ports contained. */
NUMBER_OF_TOTAL_CONNECTORS(true),
- /** Key for the metric counting the total amount of input ports contained */
+ /** Key for the metric counting the total amount of input ports contained. */
NUMBER_OF_TOTAL_ENTRY_CONNECTORS(true),
- /** Key for the metric counting the total amount of output ports contained */
+ /** Key for the metric counting the total amount of output ports contained. */
NUMBER_OF_TOTAL_EXIT_CONNECTORS(true),
- /** Key for the metric counting the total amount of elements contained */
+ /** Key for the metric counting the total amount of elements contained. */
NUMBER_OF_TOTAL_ELEMENTS(true),
- /** Key for the metric counting the total amount of elements which can be commented */
+ /** Key for the metric counting the total amount of elements which can be commented. */
NUMBER_OF_TOTAL_COMMENTABLE_ELEMENTS(true),
- /** Key for the metric counting the total amount of elements which are commented */
+ /** Key for the metric counting the total amount of elements which are commented. */
NUMBER_OF_TOTAL_COMMENTED_ELEMENTS(true),
- /**
- * Key for the metric counting the total amount of elements which do not contain other elements
- */
+ /** Key for the metric counting the total amount of elements which do not contain other elements. */
NUMBER_OF_TOTAL_LEAF_ELEMENTS(true),
- /**
- * Key for the metric counting of the total amount of constant values in all code specifications
- */
+ /** Key for the metric counting of the total amount of constant values in all code specifications. */
NUMBER_OF_TOTAL_CODE_CONSTANTS(true),
-
- /** Cyclomatic complexity number as sum over all components */
+ /** Cyclomatic complexity number as sum over all components. */
NUMBER_OF_TOTAL_CODE_CYCLOMATIC_COMPLEXITY(true),
-
- /** Counts the Lines of Code (LoC) in the component and its sub-components */
+ /** Counts the Lines of Code (LoC) in the component and its sub-components. */
NUMBER_OF_TOTAL_CODE_LINES(true),
-
- /** Representation of the total amount of comment blocks in all code specifications */
+ /** Representation of the total amount of comment blocks in all code specifications. */
NUMBER_OF_TOTAL_CODE_COMMENTS(true);
/** Metrics which are a combination from the data of multiple nodes */
@@ -106,7 +89,15 @@ public enum MetricKey {
/** Getter for {@link #collectorKeys}. */
public static List<MetricKey> getCollectorKeys() {
- return collectorKeys;
+ if (collectorKeys == null) {
+ collectorKeys = new ArrayList<>();
+ for (MetricKey key : MetricKey.values()) {
+ if (key.isCollectorKey) {
+ collectorKeys.add(key);
+ }
+ }
+ }
+ return collectorKeys;
}
/** Used for populating the {@link MetricKey#getCollectorKeys()} method. */
@@ -116,6 +107,7 @@ public enum MetricKey {
// only doubles and strings. What about marking the type of a MetricKey with a
// Class<? extends Number> field? In the constructor, you can check that only the admissible
// types Integer, Double, String are passed.
+ // TODO (#4333)
/** Determines if the corresponding value to this key is a string or not */
private boolean isStringValue;
/** Determines if the corresponding value to this key is a integer or not */
@@ -179,14 +171,8 @@ public enum MetricKey {
return isDoubleValue;
}
- // TODO (SB): It would be less bloat and easier to read to do a lazy initialization of
- // collectorKeys in getCollectorKeys(). I.e., do not initialize field with empty list, but
- // do a null check and set collectorKeys when not already done.
+ /** Get all collector keys. */
static {
- for(MetricKey key : MetricKey.values()) {
- if(key.isCollectorKey) {
- collectorKeys.add(key);
- }
- }
+ collectorKeys = getCollectorKeys();
}
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricTreeNode.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricTreeNode.java
index 0e289fb25..8eb3c6761 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricTreeNode.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricTreeNode.java
@@ -33,15 +33,14 @@ public class MetricTreeNode {
/** stores the children of this tree node */
private List<MetricTreeNode> children;
- // TODO (SB): Rename to integerMetrics (also getter). Same for doubles and strings
/** Map containing all metrics which are a Integer */
- private Map<MetricKey, Integer> storedIntegers;
+ private Map<MetricKey, Integer> integerMetrics;
/** Map containing all metrics which are a Double */
- private Map<MetricKey, Double> storedDoubles;
+ private Map<MetricKey, Double> doubleMetrics;
/** Map containing all metrics which are a String */
- private Map<MetricKey, String> storedStrings;
+ private Map<MetricKey, String> stringMetrics;
/** name of the element which is associated with this data */
private String name;
@@ -49,41 +48,39 @@ public class MetricTreeNode {
/** Constructor. */
public MetricTreeNode() {
children = new ArrayList<>();
- storedIntegers = new HashMap<>();
- storedDoubles = new HashMap<>();
- storedStrings = new HashMap<>();
+ integerMetrics = new HashMap<>();
+ doubleMetrics = new HashMap<>();
+ stringMetrics = new HashMap<>();
}
- /** @return the children of this node */
+ /** Getter for {@link #children}. */
public List<MetricTreeNode> getChildren() {
return children;
}
- /** @return the name of the element which is associated with this data */
+ /** Getter for {@link #name}. */
public String getName() {
return name;
}
- /** Set the name of this node. */
+ /** Setter for {@link #name}. */
public void setName(String name) {
this.name = name;
}
- // TODO (SB): In addition to renaming (see above), adjust comment: "the {@link Integer} metric
- // map". (Hint: auto-completing types also works in comments).
- /** @return the stored Integer map */
+ /** Getter for {@link #integerMetrics}. */
public Map<MetricKey, Integer> getStoredIntegers() {
- return storedIntegers;
+ return integerMetrics;
}
- /** @return the stored Doubles map */
+ /** Getter for {@link #doubleMetrics}. */
public Map<MetricKey, Double> getStoredDoubles() {
- return storedDoubles;
+ return doubleMetrics;
}
- /** @return the stored Strings map */
+ /** Getter for {@link #stringMetrics}. */
public Map<MetricKey, String> getStoredStrings() {
- return storedStrings;
+ return stringMetrics;
}
/**
@@ -97,9 +94,9 @@ public class MetricTreeNode {
*/
public Double getValueAsDouble(MetricKey key) {
if(key.isDoubleValue()) {
- return storedDoubles.get(key);
+ return doubleMetrics.get(key);
}
- Integer val = storedIntegers.get(key);
+ Integer val = integerMetrics.get(key);
return val == null ? null : val.doubleValue();
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/.ratings b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/.ratings
index d882fa6e7..4f07e9c17 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/.ratings
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/.ratings
@@ -1,2 +1,2 @@
-IModelQualityService.java 9a844024d62c79421e39d38699333587bec187d7 GREEN
-ModelQualityService.java 7d0021f1cd1281fa599f615b425bc9fd4e372f91 RED
+IModelQualityService.java e474e51bec8ce03209b69a40f3433a070bd73daa YELLOW
+ModelQualityService.java 4a620f5c7732069d78b0e37e051ff99f4732642e YELLOW
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/IModelQualityService.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/IModelQualityService.java
index 9a844024d..e474e51be 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/IModelQualityService.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/IModelQualityService.java
@@ -18,6 +18,8 @@ package org.fortiss.tooling.ext.quality.service;
import org.eclipse.emf.ecore.EObject;
import org.fortiss.tooling.ext.quality.IMetricProvider;
import org.fortiss.tooling.ext.quality.IMetricUpdateListener;
+import org.fortiss.tooling.ext.quality.data.MetricData;
+import org.fortiss.tooling.ext.quality.storage.ModelQualityStorageManager;
import org.fortiss.tooling.kernel.extension.data.ITopLevelElement;
/**
@@ -30,6 +32,9 @@ public interface IModelQualityService {
static IModelQualityService getInstance() {
return ModelQualityService.getInstance();
}
+
+ /** Starting the service. */
+ void startService();
/** Registers the metric provider with the service. */
<T extends EObject> void registerMetricProvider(IMetricProvider<T> provider,
@@ -38,6 +43,9 @@ public interface IModelQualityService {
/** Schedules an analysis of the metrics and processes them. */
void scheduleMetricCollection(ITopLevelElement top);
+ /** Returns the {@link MetricData} for the Service. */
+ MetricData getMetricData();
+
/**
* Registers the provided {@link IMetricUpdateListener} to be called when the metrics are
* updated.
@@ -46,4 +54,12 @@ public interface IModelQualityService {
* to be registered
*/
void registerMetricUpdateListener(IMetricUpdateListener listener);
+
+ /** Creating the quality folder to store the metrics in. */
+ static void createQualityFolder() {
+ if(!ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR.exists()) {
+ ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR.mkdirs();
+ }
+ }
+
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/ModelQualityService.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/ModelQualityService.java
index 7d0021f1c..4a620f5c7 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/ModelQualityService.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/ModelQualityService.java
@@ -31,6 +31,7 @@ import java.util.Optional;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
+import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
@@ -42,7 +43,7 @@ import org.fortiss.tooling.ext.quality.GraphMetricsProvider;
import org.fortiss.tooling.ext.quality.IMetricProvider;
import org.fortiss.tooling.ext.quality.IMetricUpdateListener;
import org.fortiss.tooling.ext.quality.data.DataRootElement;
-import org.fortiss.tooling.ext.quality.data.MetricDataManager;
+import org.fortiss.tooling.ext.quality.data.MetricData;
import org.fortiss.tooling.ext.quality.data.MetricKey;
import org.fortiss.tooling.ext.quality.data.MetricTreeNode;
import org.fortiss.tooling.ext.quality.storage.CSVFileWriter;
@@ -66,35 +67,19 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
* @author blaschke
* @author groh
*/
-// TODO (SB): Change public to /* package */ to ensure use of IModelQualityService interface
-public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider<? extends EObject>>
+/* package */ class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider<? extends EObject>>
implements IIntrospectiveKernelService, IModelQualityService, IPersistencyServiceListener {
/** The singleton instance. */
private static final ModelQualityService INSTANCE = new ModelQualityService();
/** Returns singleton instance of the service. */
- // TODO (SB): Change public to /* package */ to ensure use of IModelQualityService interface
- public static ModelQualityService getInstance() {
+ /* package */ static ModelQualityService getInstance() {
return INSTANCE;
}
- // TODO (SB): This service does not use the Eclipse extension points. Remove these 3 constants
- // and have the respective methods return "null"
- /** The connector extension point ID. */
- private static final String EXTENSION_POINT_NAME =
- "org.fortiss.tooling.ext.quality.metricService";
-
- /** The connector configuration element name. */
- private static final String CONFIGURATION_ELEMENT_NAME = "metricService";
-
- /** The connector attribute name. */
- private static final String ATTRIBUTE_NAME = "metricService";
-
- // TODO (SB): Rename class name, this field, getter, etc. to something like "MetricData". It is
- // not a manager, but a data container
/** Instance of the MetricDataManager. */
- private MetricDataManager metricDataManagerInstance = new MetricDataManager();
+ private MetricData metricDataContainer = new MetricData();
/** List of {@link IMetricUpdateListener} which are called when the metrics are updated */
private List<IMetricUpdateListener> metricUpdateListeners = new ArrayList<>();
@@ -108,9 +93,6 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
@Override
protected IStatus run(IProgressMonitor monitor) {
- // TODO (SB): Please check if the following comment that explains the loop logic is
- // correct (otherwise consider changing to while (!queuedProcessableElements.isEmpty())
- //
// Using a do-while-true loop with internal termination in order to ensure that the
// monitor is checked for cancellation at least once.
do {
@@ -127,6 +109,8 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
e.printStackTrace();
} catch(IOException e) {
e.printStackTrace();
+ } catch (CoreException e) {
+ e.printStackTrace();
}
} while(true);
}
@@ -136,7 +120,6 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
@Override
public void scheduleMetricCollection(ITopLevelElement element) {
queuedProcessableElements.add(element);
- // TODO (SB): Please check if the following comment is correct
// The Eclipse Job ensures that at most one instance of the metrics collection is executed.
// When a new element is added to the processing queue while a job is active, this current
// job will run longer in order to process it.
@@ -170,7 +153,7 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
"\n\nThis service allows to track the evolution of model metrics." +
"\n The service determines the current metrics of a model and stores" +
"\n them in a CSV file for later analysis of the modeling history." +
- "\n\nThe service extension point is '" + EXTENSION_POINT_NAME + "'.";
+ "\n\nThe service extension point is 'not existant'.";
}
/**
@@ -180,10 +163,11 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
* the {@link ITopLevelElement} on which the analysis should be performed
* @throws IOException
* @throws NoSuchAlgorithmException
+ * @throws CoreException
*/
private void performMetricCollection(ITopLevelElement topLevelElement)
- throws NoSuchAlgorithmException, IOException {
- // TODO (SB): Is the problem that metrics a duplicated when following (non-containment)
+ throws NoSuchAlgorithmException, IOException, CoreException {
+ // TODO (#4333): Is the problem that metrics a duplicated when following (non-containment)
// EReferences?
// Is is only a problem when IProjectRoot elements (as for the allocation tables) are
// referenced, or a general problem? A more general solution would be to adapt the model
@@ -192,7 +176,7 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
// Sort root elements, as some metrics are saved into trees from other root elements
// To avoid these problems, the elements have to be processed in a certain order
- // TODO (SB): In case the model traversal strategy cannot be changed, the implementation
+ // TODO (#4333): In case the model traversal strategy cannot be changed, the implementation
// below has to be generalized such that no knowledge about the AF3 metamodel is injected
// into the kernel.
@@ -216,6 +200,7 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
return 0;
});
+ var timestamp = LocalDateTime.now();
Map<IProjectRootElement, DataRootElement> updatedElements = new HashMap<>();
for(IProjectRootElement rootElement : rootElements) {
MetricTreeNode rootNode = new MetricTreeNode();
@@ -237,7 +222,7 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
.stream().collect(groupingBy(IConstraintViolation::getSource)).entrySet()) {
MetricTreeNode node =
- metricDataManagerInstance.getTreeNodeLookupTable().get(entry.getKey());
+ metricDataContainer.getTreeNodeLookupTable().get(entry.getKey());
if(node != null) {
// Collect violations and store them in the error and warning metric
@@ -250,14 +235,12 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
}
}
- // TODO (SB): I would remember the current timestamp once (i.e., before the outermost
- // for-loop). That way, all metrics of the current run will have the same timestamp. If
- // there is a reason to provide a current timestamp, you should provide a comment
+ // building the data Root Element
DataRootElement dataRootElement =
- new DataRootElement(LocalDateTime.now(), topLevelElement, rootNode);
- metricDataManagerInstance.getDataRootElementMap().put(rootElement, dataRootElement);
+ new DataRootElement(timestamp, topLevelElement, rootNode);
+ metricDataContainer.getDataRootElementMap().put(rootElement, dataRootElement);
updatedElements.put(rootElement, dataRootElement);
- metricDataManagerInstance.getTreeNodeLookupTable().put(rootElement, rootNode);
+ metricDataContainer.getTreeNodeLookupTable().put(rootElement, rootNode);
metricUpdateListeners.forEach(l -> l.metricUpdate(rootElement));
}
// Store all currently saved metrics to the CSV file
@@ -277,7 +260,7 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
private void recursivlyCollectMetrics(MetricTreeNode node, IHierarchicElement currentElement,
int recursionLevel) {
- var manager = metricDataManagerInstance;
+ var metricData = metricDataContainer;
collectMetrics(node, currentElement);
node.getStoredIntegers().put(MetricKey.NESTING_LEVEL, recursionLevel);
@@ -295,7 +278,7 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
specificationElement.getContainedElements();
if(containedElements.size() == 1) {
// Add reference from the element to this, so the lookups works as expected
- manager.getTreeNodeLookupTable().put(specificationElement, node);
+ metricData.getTreeNodeLookupTable().put(specificationElement, node);
// There is exactly one contained element. Skip the specification element to get a
// more useful tree structure
recursivlyCollectMetrics(node, containedElements.get(0), recursionLevel);
@@ -303,7 +286,7 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
recursivlyCollectMetrics(node, specificationElement, recursionLevel);
}
// Add reference from the element to this, so the lookups works as expected
- manager.getTreeNodeLookupTable().put(currentElement, node);
+ metricData.getTreeNodeLookupTable().put(currentElement, node);
} else {
// Iterate over all children and merge values
var nodeDoubles = node.getStoredDoubles();
@@ -325,12 +308,11 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
}
}
- manager.getTreeNodeLookupTable().put(currentElement, node);
- // TODO (SB): Why is the GraphMetricsProvider a special case? If possible, we should
- // turn it into an ordinary metrics provider. Otherwise, we should create a ticket for a
- // refactoring.
- GraphMetricsProvider.calculateBetweennessCentrality(currentElement, manager, false);
- GraphMetricsProvider.calculateBetweennessCentrality(currentElement, manager, true);
+ metricData.getTreeNodeLookupTable().put(currentElement, node);
+
+ // TODO (#4332) Restructure GraphMetricsProvider in quality Plugin
+ GraphMetricsProvider.calculateBetweennessCentrality(currentElement, metricData, false);
+ GraphMetricsProvider.calculateBetweennessCentrality(currentElement, metricData, true);
node.getStoredDoubles().put(MetricKey.CLUSTERING_COEFFICIENT,
GraphMetricsProvider.calculateClusteringCoefficent(currentElement));
}
@@ -351,10 +333,7 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
List<IMetricProvider<? extends EObject>> providers = getAllMetricProviders(object);
for(IMetricProvider<? extends EObject> provider : providers) {
- // TODO (SB): A nice solution would be to change getAllMetricProviders() to return a
- // List<IMetricProvider<EObject>>. Can be postponed in a ticket with a TODO(#xyz) here
- // The provider in the list has to have a compatible method for the actual type
- // as we just collected providers with a compatible type
+ //TODO (#4331)
((IMetricProvider<EObject>)provider).collectMetrics(node, object);
}
}
@@ -373,19 +352,19 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
/** {@inheritDoc} */
@Override
protected String getExtensionPointName() {
- return EXTENSION_POINT_NAME;
+ return null;
}
/** {@inheritDoc} */
@Override
protected String getConfigurationElementName() {
- return CONFIGURATION_ELEMENT_NAME;
+ return null;
}
/** {@inheritDoc} */
@Override
protected String getHandlerClassAttribute() {
- return ATTRIBUTE_NAME;
+ return null;
}
/** {@inheritDoc} */
@@ -422,15 +401,14 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
/** {@inheritDoc} */
@Override
public void topLevelElementAdded(ITopLevelElement element) {
- // Nothing done
- // TODO (SB): I think this happens when the user copies a project to the storage view and
- // "refresh"es. Is the "nothing done" intentional? Please check if my assumption is correct.
+ // Schedule the metric collection for a newly added project
+ scheduleMetricCollection(element);
}
/** {@inheritDoc} */
@Override
public void topLevelElementRemoved(ITopLevelElement element) {
- // Nothing done
+ // Nothing done so we keep the data
}
/** {@inheritDoc} */
@@ -438,11 +416,10 @@ public class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider
public void topLevelElementContentChanged(ITopLevelElement element) {
scheduleMetricCollection(element);
}
-
- // TODO (SB): The MetricDataManager (or MetricData, after the suggested renaming) is used by
- // other classes => Expose on the IModelQualityService interface.
- /** @return the {@link MetricDataManager} for this service instance. */
- public MetricDataManager getMetricDataManagerInstance() {
- return metricDataManagerInstance;
+
+ /** {@inheritDoc} */
+ @Override
+ public MetricData getMetricData() {
+ return metricDataContainer;
}
}
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 3ccb44dac..000e059c8 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 7df3c06231037c5026ca7fa18572d935d444ebf7 RED
-ModelQualityStorageManager.java 8293f17743bdc85e2595eae99b978ed868bd029b RED
+CSVFileWriter.java 89ed8b8b05dbbe55c30bd416e9255e3896ca980c YELLOW
+ModelQualityStorageManager.java c5a6783f51f596663bf836f98abf2b23130fe180 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 7df3c0623..89ed8b8b0 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
@@ -29,14 +29,16 @@ import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;
+import org.apache.commons.io.FilenameUtils;
+import org.eclipse.core.runtime.CoreException;
import org.fortiss.tooling.ext.quality.data.DataRootElement;
import org.fortiss.tooling.ext.quality.data.MetricKey;
+import org.fortiss.tooling.ext.quality.service.IModelQualityService;
import org.fortiss.tooling.kernel.extension.data.ITopLevelElement;
import org.fortiss.tooling.kernel.model.IProjectRootElement;
@@ -47,16 +49,10 @@ import org.fortiss.tooling.kernel.model.IProjectRootElement;
*/
public class CSVFileWriter {
- // TODO (SB): Comment is unclear. What is the default CSV file (I thought there are per-project
- // metric files now)? What do you mean by the conversion?
/**
- * Method to write the data into the default CSV file.
- * <p>
- * It will always write to the same file, if it is already present it will attempt to read the
- * keys and convert them into {@link MetricKey} and then write these keys in the ordering that
- * is already present. This is done such that the log file always remains consistent, but can
- * cause newly added {@link MetricKey} not to be exported if they were not present when the file
- * was exported the first time.
+ * Method to write the data into CSV files. For each project one file is built. If it is already existant,
+ * it uses to the existing keys in the file as dimensions, to help that the log file always remains consistent.
+ * Newly added MetricKeys are not added in the next exportations if it was not present and build.
*
* @param map
* to save into CSV file
@@ -64,38 +60,31 @@ public class CSVFileWriter {
* @throws NoSuchAlgorithmException
*
*/
+
+ /** Suffix of the file in which the data is written. */
+ private static final String CSV = ".csv";
+
public static void metricExtractionToCSV(Map<IProjectRootElement, DataRootElement> map)
- throws NoSuchAlgorithmException, IOException {
+ throws NoSuchAlgorithmException, IOException, CoreException {
// check if there is any data to export
if(map == null || map.isEmpty()) {
return;
}
-
- // TODO(SB): Move functionality to IModelQualityService (to which the directory is
- // configured during startup, see comments in ModelQualityStorageManager)
- // Ensure the folder exists
- if(!ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR.exists()) {
- ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR.mkdirs();
- }
+
+ IModelQualityService.createQualityFolder();
// path to CSV file
- // TODO (SB): What about the following (avoids creating an array):
- // Check above is that iterator.next() returns at least one value
- // DataRootElement firstValue = map.entrySet().iterator().next().getValue();
- Collection<DataRootElement> values = map.values();
- DataRootElement firstValue = values.toArray(new DataRootElement[0])[0];
+ DataRootElement firstValue = map.entrySet().iterator().next().getValue();
ITopLevelElement topLevelElement = firstValue.getTopLevelElement();
String projectName = topLevelElement.getSaveableName();
- // TODO: the quality file name should be determined without knowledge of the AF3 file
- // extension. Maybe FilenameUtils.getBaseName(filename) is useful?
- Path path = new File(ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR, projectName
- .replace(ModelQualityStorageManager.AF3_PRJ_SUFFIX, ModelQualityStorageManager.CSV))
- .toPath();
+ var fileBaseName = FilenameUtils.getBaseName(projectName).toString();
+ Path path = new File(ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR, fileBaseName.concat(CSV)).toPath();
- // TODO(SB): This assumes that the file has been saved to disk.
- // Add topLevelElement.doSave(new NullProgressMonitor());
+ //save before using the saved file to produce the hash
+ //FIXME (KB): produces unstoppable exceptions if I use it (IllegalStateExceptions)
+ //topLevelElement.doSave(new NullProgressMonitor());
Path modelFile = new File(ModelQualityStorageManager.MODEL_PROJECT_DIR,
topLevelElement.getSaveableName()).toPath();
String projectHash = computeGitObjectHash(modelFile);
@@ -142,18 +131,13 @@ public class CSVFileWriter {
try(var writer = new BufferedWriter(new FileWriter(path.toFile(), true))) {
if(createNewIndex) {
- // TODO (SB): It would be save to start the (currently 6) non-value keys with the
- // same (reserved) prefix. That way, we can later change the strategy to distinguish
- // them for the value keys. The implementation below to assume that there are
- // currently 6 non-value keys is fine.
-
// Create new index and write it into the first line of the file.
- allKeys.add("timestamp");
- allKeys.add("project_name");
- allKeys.add("project_hash");
- allKeys.add("root_element_name");
- allKeys.add("name");
- allKeys.add("children");
+ allKeys.add("nv_timestamp");
+ allKeys.add("nv_project_name");
+ allKeys.add("nv_project_hash");
+ allKeys.add("nv_root_element_name");
+ allKeys.add("nv_name");
+ allKeys.add("nv_children");
// Collect all MetricKeys, convert them to a string and append them to the allKeys
// list
allKeys.addAll(Arrays.stream(MetricKey.values()).map(MetricKey::toString)
@@ -190,11 +174,12 @@ public class CSVFileWriter {
// Collect all values from provider in the correct order and combine
var values = Stream.concat(startStream, valueKeys.stream()
.map(String::toUpperCase).map(MetricKey::valueOf).map(k -> {
- // TODO (SB): Always use braces for if. Avoids future problems :-)
- if(k.isStringValue())
+ if(k.isStringValue()) {
return strings.get(k);
- if(k.isDoubleValue())
+ }
+ if(k.isDoubleValue()) {
return doubles.get(k);
+ }
return integers.get(k);
}).map(String::valueOf)).collect(Collectors.joining(","));
try {
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/ModelQualityStorageManager.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/ModelQualityStorageManager.java
index 8293f1774..c5a6783f5 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/ModelQualityStorageManager.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/ModelQualityStorageManager.java
@@ -21,10 +21,8 @@ import java.io.File;
import org.eclipse.core.resources.IWorkspaceRoot;
-// TODO (SB): This class is not really a manager, but a collection of constants.
-// TODO (SB): This class makes the quality calculation AF3 specific. The suggested
-// resolution is to distribute the information to other classes (as below).
-// Possibly postpone this in a dedicated ticket, and reference it here (TODO (#xyz))
+// TODO (#4333) Exclude the constants to the corresponding packages. The suggested
+// resolution is to distribute the information to other classes.
/**
* Manages the directory of the CSV files.
*
@@ -32,35 +30,27 @@ import org.eclipse.core.resources.IWorkspaceRoot;
*/
public class ModelQualityStorageManager {
- // TODO(SB): Move to org.fortiss.af3.project (e.g., create sister class of AF3Project (e.g.,
+ // TODO(#4333): Move to org.fortiss.af3.project (e.g., create sister class of AF3Project (e.g.,
// AF3Metrics), and configure the IModelQualityService from there with the directory name, etc.)
/** The name of the general quality plugin directory. */
public static final String AF3_QUALITY_DIRECTORY_NAME = "AF3-Model-Quality-Directory";
- // TODO(SB): Remove! The single source of truth for this is
+ // TODO(#4333): Remove! The single source of truth for this is
// org.fortiss.af3.project.utils.FileUtils
/** The name of the general project directory. */
public static final String AF3_PROJECT_DIRECTORY_NAME = "AF3-Project-Directory";
- // TODO(SB): Remove (unneeded magic)
+ // TODO(#4333): Remove (unneeded magic)
/** The current workspace root. */
public static final IWorkspaceRoot WORKSPACE_ROOT = getWorkspace().getRoot();
- // TODO(SB): Remove, see suggested AF3Metrics class above
+ // TODO(#4333): Remove, see suggested AF3Metrics class above
/** {@link File} representing the directory where all data is saved. */
public static final File MODEL_QUALITY_PROJECT_DIR =
new File(WORKSPACE_ROOT.getLocation() + File.separator + AF3_QUALITY_DIRECTORY_NAME);
- // TODO(SB): Remove, see suggested AF3Metrics class above
+ // TODO(#4333): Remove, see suggested AF3Metrics class above
/** {@link File} representing the directory where all data is saved. */
public static final File MODEL_PROJECT_DIR =
new File(WORKSPACE_ROOT.getLocation() + File.separator + AF3_PROJECT_DIRECTORY_NAME);
-
- // TODO (SB): Remove! The single source of truth for this is org.fortiss.af3.project.AF3Project
- /** Defines the suffix of AF3 project files. */
- public static final String AF3_PRJ_SUFFIX = "af3_23";
-
- // TODO(SB): Move as (private) constant CSV_SUFFIX to CSVFileWriter
- /** Suffix of the file in which the data is written. */
- public static final String CSV = "csv";
}
--
GitLab