Skip to content
Snippets Groups Projects
Commit 0496ec4e authored by blaschke's avatar blaschke
Browse files

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: af3#4310


Signed-off-by: default avatarKonstantin Blaschke <blaschke@fortiss.org>
parent 249c9a26
No related branches found
No related tags found
1 merge request!210Setting up Metric extraction plugin for AF3 : Issue 4310
Pipeline #39279 failed
Stage: test
Hide whitespace changes
Inline Side-by-side
Showing
with 303 additions and 468 deletions
org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/.ratings
+ 1
− 1
View file @ 0496ec4e
ModelQualityExtractionMenu.java e2926b47f4f30adb160c47aef773853bb5b03fcb RED
ModelQualityExtractionMenu.java 3bdbf3d37715539648af8ba7feefbec9370c4dc4 YELLOW
ModelQualityUIActivator.java 2f47db82bc088072dbfe53f82d3ed417d77fd23c GREEN
org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/ModelQualityExtractionMenu.java
+ 4
− 8
View file @ 0496ec4e
......@@ -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;
}
......
org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/.ratings
+ 2
− 2
View file @ 0496ec4e
IModelQualityViewPart.java 708f8089645df12098ea67190805cce343045d2e GREEN
ModelQualityFXController.java f4ae42467509d29326bdabe822310083253910dc RED
ModelQualityFXViewPart.java 4f142d1b04ab9e5f0ce54ff248fe0abf6f141937 RED
ModelQualityFXController.java 67bb0a067c578ac9a9b30e5973587cc57374a782 YELLOW
ModelQualityFXViewPart.java a1e82fe68a3fe7d813fa61d5148ffcd1592000f7 YELLOW
org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXController.java
+ 19
− 21
View file @ 0496ec4e
......@@ -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: " +
......
org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXViewPart.java
+ 2
− 0
View file @ 0496ec4e
......@@ -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();
......
org.fortiss.tooling.ext.quality/META-INF/MANIFEST.MF
+ 1
− 1
View file @ 0496ec4e
......@@ -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,
......
org.fortiss.tooling.ext.quality/html/developer/.ratings
+ 1
− 1
View file @ 0496ec4e
documentation.html f102e6b3757eec158de9bf78238e2865d01de2a4 RED
documentation.html b66c2bfb495047a8e4b680b02231ab2fdb34cec3 YELLOW
org.fortiss.tooling.ext.quality/html/developer/documentation.html
+ 22
− 120
View file @ 0496ec4e
......@@ -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>
......
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/.ratings
+ 3
− 3
View file @ 0496ec4e
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
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/GraphMetricsProvider.java
+ 68
− 83
View file @ 0496ec4e
......@@ -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);
......
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementProvider.java → org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementMetricProvider.java
+ 43
− 38
View file @ 0496ec4e
......@@ -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
}
}
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/AF3QualityActivator.java → org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/ModelQualityActivator.java
+ 7
− 10
View file @ 0496ec4e
......@@ -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;
}
}
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/.ratings
+ 4
− 4
View file @ 0496ec4e
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
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/DataRootElement.java
+ 8
− 20
View file @ 0496ec4e
......@@ -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;
}
......
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricDataManager.java → org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricData.java
+ 7
− 21
View file @ 0496ec4e
......@@ -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;
}
......
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricKey.java
+ 38
− 52
View file @ 0496ec4e
......@@ -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();
}
}
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricTreeNode.java
+ 17
− 20
View file @ 0496ec4e
......@@ -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();
}
......
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/.ratings
+ 2
− 2
View file @ 0496ec4e
IModelQualityService.java 9a844024d62c79421e39d38699333587bec187d7 GREEN
ModelQualityService.java 7d0021f1cd1281fa599f615b425bc9fd4e372f91 RED
IModelQualityService.java e474e51bec8ce03209b69a40f3433a070bd73daa YELLOW
ModelQualityService.java 4a620f5c7732069d78b0e37e051ff99f4732642e YELLOW
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/IModelQualityService.java
+ 16
− 0
View file @ 0496ec4e
......@@ -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();
}
}
}
org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/ModelQualityService.java
+ 38
− 61
View file @ 0496ec4e
......@@ -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;
}
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment