diff --git a/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/.ratings b/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/.ratings index a6e50fb4b234d81008daabdb059a8693810fee7f..ac820fa033fb4fdc5cb1f91ab263d5737355d778 100644 --- a/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/.ratings +++ b/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/.ratings @@ -1,8 +1,8 @@ AdvancedTreeViewerEditorBase.java 3bb67c52886069379765289885df3b23790b88c9 GREEN AllocationDiagramEditorBase.java 3c03b9220f1c342ded673d6d1b2fd2c01b57fba6 GREEN CommonDiagramEditorBase.java 438babd95dd7a185bd32246cb8180b00bb53f18a GREEN -ConstraintBasedProcessEditor.java f42887f36324edd3e52ffc8f9c6c574bb071a7f2 RED -ConstraintBasedProcessEditorHelper.java 2ac99a960a45b891b3fe1dbff3b79a3fc5f4f834 RED +ConstraintBasedProcessEditor.java 5b065b9e417a142c6b0a37ad75b9a41c513743e4 YELLOW +ConstraintBasedProcessEditorHelper.java 8c1e6cd5ae787199a9f0b2130302dcda5d61e347 YELLOW DiagramEditorBase.java 3d2bb40e18548ebca0dfdd78f094598e5ee298d1 GREEN DiagramKeyHandler.java 8b64048b966e6e8cacfa7fb78edebef2a4981fc4 GREEN FormsEditorBase.java 50934d36124dea9b16ac45fe3621d878afb48bc7 GREEN diff --git a/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/ConstraintBasedProcessEditor.java b/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/ConstraintBasedProcessEditor.java index 95ef24c46909dd4e9e02c59c62d188bb8086eba7..02863e93aef67ac1ac44a757a4827c57ecf408d0 100644 --- a/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/ConstraintBasedProcessEditor.java +++ b/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/ConstraintBasedProcessEditor.java @@ -181,10 +181,8 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e currentObjComposite.setLayout(new GridLayout(2, false)); } - // TODO (SB,9): Can you think of a better name that explains the purpose of a fake child. Is it - // a place holder? - /** Fake child used to add new elements. */ - private static class FakeChild { + /** Dummy object used to add new elements in the tree. */ + private static class DummyObject { // Nothing to implement: just a marker class } @@ -222,7 +220,7 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e if(parentElement instanceof IConstraintBasedProcess) { IConstraintBasedProcess cbdp = (IConstraintBasedProcess)parentElement; ArrayList<Object> configs = new ArrayList<>(); - configs.add(new FakeChild()); + configs.add(new DummyObject()); configs.addAll(cbdp.getConfigurations()); return configs.toArray(); } else if(parentElement instanceof ConstraintConfiguration) { @@ -233,15 +231,14 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e availableCstrs.removeAll(IConstraintUIService.getInstance() .getAlwaysActivatedConstraints()); - if(editedObject.getConfigurations().size() > 1) { - // TODO(SB,19) - List<ConfigurationRefWithContextualConfiguration> refs = - editedObject - .getConfigurations() - .stream() + EList<ConstraintConfiguration> configs = editedObject.getConfigurations(); + if(configs.size() > 1) { + List<ConfigurationRefWithContextualConfiguration> configCollection = + configs.stream() .map(t -> new ConfigurationRefWithContextualConfiguration(t, (ConstraintConfiguration)parentElement)) .collect(toList()); + List<ConfigurationRefWithContextualConfiguration> refs = configCollection; refs.removeIf(c -> c.config.getName().equalsIgnoreCase(c.target.getName())); refs.forEach(c -> { parentList.add(new TreeStructureNode(parentElement, c)); @@ -299,8 +296,6 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e HashMap<String, String> groupedConstraint = groupNameAndConstraints.get(constraintGroupName); for(String key : groupedConstraint.keySet()) { - // TODO(SB,1) - // if(isUniqueConstraint(key)) { constraintInfo.put(key, groupedConstraint.get(key)); TreeStructureNode node = new TreeStructureNode(parentElement, key); parentList.add(node); @@ -324,7 +319,7 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e /** Text labels for the tree of configurations. */ private String getLabelText(Object element) { - if(element instanceof FakeChild) { + if(element instanceof DummyObject) { return "click to add a new configuration"; } else if(element instanceof ConstraintConfiguration) { return ((ConstraintConfiguration)element).getName(); @@ -369,8 +364,6 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e (ConstraintWithContextualConfiguration)element; IConstraintService cs = IConstraintService.getInstance(); String name = cs.getName(cstrInContext.cstr); - // TODO(SB,1) - // return cstrInContext.config.getActiveConstraints().contains(name); HashMap<String, String> hashMap = groupNameAndConstraints.get(element.toString()); return hashMap.keySet().contains(name); } else if(element instanceof ConfigurationRefWithContextualConfiguration) { @@ -467,15 +460,17 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e List<ConstraintConfiguration> activeConfigs = getActiveConfigurationsTransitively(config, null); // If the selected constraint is in the active constraints list - // TODO(SB,14): Provide reason why get(0) cannot fail. Additional - // test might be needed. - EList<String> activeConstraints = - activeConfigs.get(0).getActiveConstraints(); - if(activeConstraints.contains(cs - .getConstraintByName(constraintName).getCanonicalName())) { - // Activate the constraint - cuis.activate(cs.getConstraintByName(constraintName), - cstrContainer); + if(activeConfigs.get(0) != null) { + // get all active constraints of the first configuration which + // is selected using activeConfigs.get(0) + EList<String> activeConstraints = + activeConfigs.get(0).getActiveConstraints(); + if(activeConstraints.contains(cs.getConstraintByName( + constraintName).getCanonicalName())) { + // Activate the constraint + cuis.activate(cs.getConstraintByName(constraintName), + cstrContainer); + } } } else { clearConstraintInstance(cs, cuis, config, constraintName); @@ -521,7 +516,7 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e /** {@inheritDoc} */ @Override protected void setValue(Object element, Object value) { - if(element instanceof FakeChild) { + if(element instanceof DummyObject) { if(!alreadyExists((String)value)) { ConstraintConfiguration newconfig = ElementFactory.eINSTANCE.createConstraintConfiguration(); @@ -558,8 +553,8 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e /** {@inheritDoc} */ @Override protected boolean canEdit(Object element) { - return element instanceof ConstraintConfiguration || element instanceof FakeChild || - constraintMap.containsKey(element); + return element instanceof ConstraintConfiguration || + element instanceof DummyObject || constraintMap.containsKey(element); } }); treeViewer.setUseHashlookup(true); @@ -628,9 +623,9 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e /** {@inheritDoc} */ @Override public int compare(Viewer viewer, Object e1, Object e2) { - if(e1 instanceof FakeChild) { + if(e1 instanceof DummyObject) { return -1; - } else if(e2 instanceof FakeChild) { + } else if(e2 instanceof DummyObject) { return 1; } else if(e1 instanceof ConstraintConfiguration && e2 instanceof ConstraintConfiguration) { @@ -701,8 +696,10 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e // Not supported } - // TODO(SB, 4): Explain what the method does /** + * If the constraint is unchecked from the tree remove the + * constraint from the active constraints list and constraint instance list + * * @param cs * constraintService * @param cuis @@ -714,8 +711,6 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e */ private void clearConstraintInstance(IConstraintService cs, IConstraintUIService cuis, ConstraintConfiguration config, String constraintName) { - // If the constraint is unchecked from the tree remove the - // constraint from the active constraints list and constraint instance list config.getActiveConstraints().remove(constraintName); // If the constraint is still not active @@ -773,8 +768,6 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e ConstraintConfiguration config = ((ConstraintConfiguration)provider.getParent(parentElement)); - // TODO(SB,1) - // for(TreeStructureNode node : parentList) { List<Class<? extends IConstraint>> activeConstraintsTransitively = getActiveConstraintsTransitively(config, null); List<String> constraintList = @@ -827,16 +820,16 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e } } } - // TODO (SB,9): I think the Integer.toString() can be omitted? - return "[ " + Integer.toString(successCount) + " / " + Integer.toString(failCount) + - "/" + Integer.toString(hashMap.size() - duplicateSize) + " ]"; + + int totalNumOfGrpConstraints = hashMap.size() - duplicateSize; + return "[ " + successCount + " / " + failCount + "/" + totalNumOfGrpConstraints + " ]"; } return ""; } /** Text labels for the tree of configurations. */ private StyledString getStylizedLabel(Object element) { - if(element instanceof FakeChild) { + if(element instanceof DummyObject) { return new StyledString("click to add a new configuration"); } else if(element instanceof ConstraintConfiguration) { return new StyledString(((ConstraintConfiguration)element).getName()); @@ -864,26 +857,28 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e finalString.setStyle(0, finalString.length(), new NormalFontStyler()); return finalString; } - // TODO (SB, 14): Add check that instanceCountList has at least 3 elements // the elements are stored in the list sequentially- success count, fail - // count and total of instances - int successLength = Integer.toString(instanceCountList.get(0)).length(); - int failCountLength = Integer.toString(instanceCountList.get(1)).length(); + // count and total number of instances + String successfulConst = Integer.toString(instanceCountList.get(0)); + int successLength = successfulConst.length(); + String failedConst = Integer.toString(instanceCountList.get(1)); + String totalConst = Integer.toString(instanceCountList.get(2)); + int failCountLength = failedConst.length(); // creating the string with all the counts - // TODO (SB,9): I think the Integer.toString() can be omitted? - StyledString finalString = - new StyledString(constraintGroupName + "[" + - Integer.toString(instanceCountList.get(0)) + "/" + - Integer.toString(instanceCountList.get(1)) + "/" + - Integer.toString(instanceCountList.get(2)) + "]"); - adjustColorIndex(groupNameAndConstraints.keySet().size()); - - finalString.setStyle(0, finalString.length(), new NormalFontStyler()); - finalString.setStyle(constarintNameLength + 1, successLength, - new GreenFontStyler()); - finalString.setStyle(constarintNameLength + 2 + successLength, - failCountLength, new RedFontStyler()); - return finalString; + if(instanceCountList.size() == 3) { + + StyledString finalString = + new StyledString(constraintGroupName + "[" + successfulConst + + "/" + failedConst + "/" + totalConst + "]"); + adjustColorIndex(groupNameAndConstraints.keySet().size()); + + finalString.setStyle(0, finalString.length(), new NormalFontStyler()); + finalString.setStyle(constarintNameLength + 1, successLength, + new GreenFontStyler()); + finalString.setStyle(constarintNameLength + 2 + successLength, + failCountLength, new RedFontStyler()); + return finalString; + } } } } else if(constraintMap.containsKey(stringNode)) { @@ -895,9 +890,6 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e /** Class to create stylized string for labels of the tree. */ private class BoldColumnLabelProvider extends DelegatingStyledCellLabelProvider { - // TODO(SB,1) - // Styler styler = new BoldFontStyler(); - /** {@inheritDoc} */ @Override public Color getForeground(Object element) { @@ -977,13 +969,6 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e Object parent = provider.getParent(element); ConstraintConfiguration config = ((ConstraintConfiguration)parent); - EList<String> activeConstraints = config.getActiveConstraints(); - List<Class<? extends IConstraint>> stillActiveCstrs = - getActiveConstraintsTransitively(config, null); - - int duplicateSize = - stillActiveCstrs.stream().filter(c -> !(activeConstraints.contains(c.getName()))) - .collect(toList()).size(); if(config.getName().equalsIgnoreCase(editedObject.getCurrentObjective().getName())) { for(ConstraintInstance instance : constraintInstances) { for(String constraintName : editedObject.getCurrentObjective() @@ -1004,7 +989,7 @@ public class ConstraintBasedProcessEditor<CBP extends IConstraintBasedProcess> e ArrayList<Integer> countList = new ArrayList<Integer>(); countList.add(new Integer(successCount)); countList.add(new Integer(failCount)); - countList.add(new Integer(hashMap.size() - duplicateSize)); + countList.add(new Integer(hashMap.size())); return countList; } return null; diff --git a/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/ConstraintBasedProcessEditorHelper.java b/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/ConstraintBasedProcessEditorHelper.java index 44c6d8aebd6bf3465d6d17f0437b7658786daef5..9450763e8b9c90db8c49249faf855a84c70cac48 100644 --- a/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/ConstraintBasedProcessEditorHelper.java +++ b/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/editor/ConstraintBasedProcessEditorHelper.java @@ -27,10 +27,15 @@ import org.eclipse.swt.widgets.Display; import org.fortiss.tooling.base.model.element.ConstraintConfiguration; import org.fortiss.tooling.kernel.extension.IConstraint; -// TODO(SB,3): Add class comment, use proper author name tag (fortiss login) /** * - * @author ss + * This class is a helper class that provides, 1) the colors for the constraint groups, + * 2) manage the references between the constraint configurations and + * 3) manages the tree manipulation that is used to visualize the constraintsGroups, + * constraintConfigurations and constraints. + * + * @author abid + * */ public class ConstraintBasedProcessEditorHelper { @@ -75,8 +80,7 @@ public class ConstraintBasedProcessEditorHelper { WithContextualConfiguration { /** Constraint. */ - // TODO: Check if the variable should really be package accessible - /* package */ConstraintConfiguration target; + public ConstraintConfiguration target; /** Constructor. */ public ConfigurationRefWithContextualConfiguration(ConstraintConfiguration configRef, @@ -89,13 +93,18 @@ public class ConstraintBasedProcessEditorHelper { @Override public boolean equals(Object obj) { if(obj instanceof ConfigurationRefWithContextualConfiguration) { - // TODO (SB, 19) - return this.target.getName().equalsIgnoreCase( - ((ConfigurationRefWithContextualConfiguration)obj).target.getName()) && - this.config.getName() - .equalsIgnoreCase( - ((ConfigurationRefWithContextualConfiguration)obj).config - .getName()); + // current target name + String currentTargetName = this.target.getName(); + // contextualRef Target of the configuration + String configRefTrgtObjName = + ((ConfigurationRefWithContextualConfiguration)obj).target.getName(); + // current configuration + String currentConfigName = this.config.getName(); + // contextualRef Object of the configuration + String configRefObjName = + ((ConfigurationRefWithContextualConfiguration)obj).config.getName(); + return currentTargetName.equalsIgnoreCase(configRefTrgtObjName) && + currentConfigName.equalsIgnoreCase(configRefObjName); } return false; } @@ -110,12 +119,11 @@ public class ConstraintBasedProcessEditorHelper { /** Class to manipulate and store parent of elements. */ public static class TreeStructureNode { - // TODO: Check if the variables should really be package accessible /** Parent object variable. */ - /* package */Object parent; + public Object parent; /** Data object variable. */ - /* package */Object data; + public Object data; /** Constructor. */ public TreeStructureNode(Object parent, Object children) { @@ -127,11 +135,16 @@ public class ConstraintBasedProcessEditorHelper { @Override public boolean equals(Object obj) { if(obj instanceof TreeStructureNode) { - // TODO (SB, 19) - return this.data.toString().equalsIgnoreCase( - ((TreeStructureNode)obj).data.toString()) && - this.parent.toString().equalsIgnoreCase( - ((TreeStructureNode)obj).parent.toString()); + // the current tree node to be compared with + String currentStructureNode = this.data.toString(); + // the tree node to be checked + String treeStructureNode = ((TreeStructureNode)obj).data.toString(); + // parent of the current tree node + String currentParentNode = this.parent.toString(); + // parent of the object that needs to be checked + String objectParentNode = ((TreeStructureNode)obj).parent.toString(); + return currentStructureNode.equalsIgnoreCase(treeStructureNode) && + currentParentNode.equalsIgnoreCase(objectParentNode); } return false; } @@ -147,8 +160,7 @@ public class ConstraintBasedProcessEditorHelper { public static class NormalFontStyler extends Styler { /** Counter to keep track of currently used color in color array. */ - // TODO: Check if the variable should really be package accessible - /* package */int colorIndex = 0; + public int colorIndex = 0; /** Constructor. */ public NormalFontStyler() { @@ -166,8 +178,7 @@ public class ConstraintBasedProcessEditorHelper { } } - // TODO (SB,3) - /** */ + /** Styler class to create red color labels. */ public static class RedFontStyler extends Styler { /** {@inheritDoc} */ @Override @@ -191,15 +202,18 @@ public class ConstraintBasedProcessEditorHelper { } } - // TODO(SB,3) /** + * This function is responsible for updating the color of the constraints groups as they are + * added by updating the color counter (i.e., ColorCtr) + * * @param groupSize - * the size of the groups present + * is the size of the constraints groups */ public static void adjustColorIndex(int groupSize) { - // TODO(SB,11): Can you use modulo to write this more elegantly? - if(ConstraintBasedProcessEditorHelper.colorCtr != ConstraintBasedProcessEditorHelper.colorCtr - 1) { - if(groupSize == ConstraintBasedProcessEditorHelper.colorCtr + 1) { + int colorIndex = ConstraintBasedProcessEditorHelper.colorCtr; + + if(colorIndex != colorIndex - 1) { + if(groupSize == colorIndex + 1) { ConstraintBasedProcessEditorHelper.colorCtr = 0; } else { ConstraintBasedProcessEditorHelper.colorCtr++; diff --git a/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/utils/StatusUtils.java b/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/utils/StatusUtils.java deleted file mode 100644 index f7bf3ce6e47c14a90ffdd6a1e25d041af0267df3..0000000000000000000000000000000000000000 --- a/org.fortiss.tooling.base.ui/trunk/src/org/fortiss/tooling/base/ui/utils/StatusUtils.java +++ /dev/null @@ -1,65 +0,0 @@ -/*-------------------------------------------------------------------------+ -| Copyright 2011 fortiss GmbH | -| | -| Licensed under the Apache License, Version 2.0 (the "License"); | -| you may not use this file except in compliance with the License. | -| You may obtain a copy of the License at | -| | -| http://www.apache.org/licenses/LICENSE-2.0 | -| | -| Unless required by applicable law or agreed to in writing, software | -| distributed under the License is distributed on an "AS IS" BASIS, | -| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | -| See the License for the specific language governing permissions and | -| limitations under the License. | -+--------------------------------------------------------------------------*/ -package org.fortiss.tooling.base.ui.utils; - -import org.eclipse.emf.common.util.EList; -import org.fortiss.tooling.kernel.model.constraints.ConstraintInstance; -import org.fortiss.tooling.kernel.model.constraints.FailedConstraintInstanceStatus; -import org.fortiss.tooling.kernel.model.constraints.IConstraintInstanceContainer; - -// TODO (SB, 9): The class name is misleading (focus is on constraints here). Further, areAllInstancesSuccessful() is not used anywhere -> remove altogether? -/** - * Methods for accessing the status line. - * - * @author gareis - */ -public class StatusUtils { - - /** - * This function is to calculate the status of all of the instances of a particular constraint. - * - * @param constraintToFind - * name of the constraint - * @param instanceContainer - * contains the instance container for the current constraints that are active - * @param currentActiveConstraints - * contains the names of the current active constraints - * @return true or false based on the status of the constraint (Failed or Success) - */ - - public static boolean areAllInstancesSuccessful(String constraintToFind, - IConstraintInstanceContainer instanceContainer, EList<String> currentActiveConstraints) { - - for(ConstraintInstance ci : instanceContainer.getConstraintInstances()) { - if(constraintToFind.equalsIgnoreCase(ci.getConstraintName())) { - // TODO (SB,19): toString() not needed for String? - // TODO(SB,19): Combine these 3 nested if's into one (using appropriately named - // boolean variables) - if(currentActiveConstraints.contains(constraintToFind.toString())) { - // TODO (SB,19): toString() not needed for String? - if(ci.getConstraintName().equalsIgnoreCase(constraintToFind.toString())) { - // If the constraint instance has a "failed status" - if(ci.getStatus() instanceof FailedConstraintInstanceStatus) { - return false; - } - } - } - return true; - } - } - return false; - } -}