prmr#416 refactor edge constraints checking

ConstraintSet is now stored as a private static final field in each diagram builder. This reduces object creation by eliminating the need to create and merge new ConstraintSet objects each time canAdd is called. For this reason, I removed the `merge` method in ConstraintSet which is no longer necessary. The ConstraintSet field can be accessed by method getEdgeConstraints() which simply returns the set of constraints. I see no encapsulation issue in doing so since there is no method available to modify contents of a ConstraintSet which will be returned. Satisfied takes in the canAdd parameters in order to check the conditions while keeping the code simple.
louib · Aug 15, 2021 · 20fd32b · 20fd32b
1 parent 083cc96
commit 20fd32b
Show file tree

Hide file tree

Showing 20 changed files with 259 additions and 261 deletions.
diff --git a/.classpath b/.classpath
@@ -1,23 +1,23 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<classpath>
-	<classpathentry kind="src" path="src"/>
-	<classpathentry kind="src" path="test"/>
-	<classpathentry kind="src" path="icons"/>
-	<classpathentry kind="src" path="tipdata"/>
-	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER">
-		<attributes>
-			<attribute name="module" value="true"/>
-		</attributes>
-	</classpathentry>
-	<classpathentry kind="con" path="org.eclipse.jdt.USER_LIBRARY/JavaFX14">
-		<attributes>
-			<attribute name="module" value="true"/>
-		</attributes>
-	</classpathentry>
-	<classpathentry kind="con" path="org.eclipse.jdt.junit.JUNIT_CONTAINER/5">
-		<attributes>
-			<attribute name="module" value="true"/>
-		</attributes>
-	</classpathentry>
-	<classpathentry kind="output" path="bin/jetuml"/>
-</classpath>
+<?xml version="1.0" encoding="UTF-8"?>
+<classpath>
+	<classpathentry kind="src" path="src"/>
+	<classpathentry kind="src" path="test"/>
+	<classpathentry kind="src" path="icons"/>
+	<classpathentry kind="src" path="tipdata"/>
+	<classpathentry kind="con" path="org.eclipse.jdt.USER_LIBRARY/JavaFX14">
+		<attributes>
+			<attribute name="module" value="true"/>
+		</attributes>
+	</classpathentry>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER">
+		<attributes>
+			<attribute name="module" value="true"/>
+		</attributes>
+	</classpathentry>
+	<classpathentry kind="con" path="org.eclipse.jdt.junit.JUNIT_CONTAINER/5">
+		<attributes>
+			<attribute name="module" value="true"/>
+		</attributes>
+	</classpathentry>
+	<classpathentry kind="output" path="bin/jetuml"/>
+</classpath>
diff --git a/src/ca/mcgill/cs/jetuml/diagram/builder/ClassDiagramBuilder.java b/src/ca/mcgill/cs/jetuml/diagram/builder/ClassDiagramBuilder.java
@@ -27,7 +27,6 @@
 
 import ca.mcgill.cs.jetuml.diagram.Diagram;
 import ca.mcgill.cs.jetuml.diagram.DiagramType;
-import ca.mcgill.cs.jetuml.diagram.Edge;
 import ca.mcgill.cs.jetuml.diagram.Node;
 import ca.mcgill.cs.jetuml.diagram.builder.constraints.ClassDiagramEdgeConstraints;
 import ca.mcgill.cs.jetuml.diagram.builder.constraints.ConstraintSet;
@@ -49,7 +48,19 @@ public class ClassDiagramBuilder extends DiagramBuilder
 {
 	private static final int PADDING = 10;
 	private static final int TOP_HEIGHT = 20;
-
+	private static final ConstraintSet constraints = new ConstraintSet(
+			EdgeConstraints.noteEdge(),
+			EdgeConstraints.noteNode(),
+			EdgeConstraints.maxEdges(1),
+			ClassDiagramEdgeConstraints.noSelfGeneralization(),
+			ClassDiagramEdgeConstraints.noSelfDependency(),
+			ClassDiagramEdgeConstraints.noDirectCycles(DependencyEdge.class),
+			ClassDiagramEdgeConstraints.noDirectCycles(GeneralizationEdge.class),
+			ClassDiagramEdgeConstraints.noDirectCycles(AggregationEdge.class),
+			ClassDiagramEdgeConstraints.noDirectCycles(AssociationEdge.class),
+			ClassDiagramEdgeConstraints.noCombinedAssociationAggregation()
+	);
+
 	/**
 	 * Creates a new builder for class diagrams.
 	 * 
@@ -101,18 +112,9 @@ public DiagramOperation createAddNodeOperation(Node pNode, Point pRequestedPosit
 	}
 
 	@Override
-	protected ConstraintSet getAdditionalEdgeConstraints(Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint)
+	protected ConstraintSet getEdgeConstraints()
 	{
-		return new ConstraintSet(
-				EdgeConstraints.maxEdges(pEdge, pStart, pEnd, aDiagram, 1),
-				ClassDiagramEdgeConstraints.noSelfGeneralization(pEdge, pStart, pEnd),
-				ClassDiagramEdgeConstraints.noSelfDependency(pEdge, pStart, pEnd),
-				ClassDiagramEdgeConstraints.noDirectCycles(DependencyEdge.class, pEdge, pStart, pEnd),
-				ClassDiagramEdgeConstraints.noDirectCycles(GeneralizationEdge.class, pEdge, pStart, pEnd),
-				ClassDiagramEdgeConstraints.noDirectCycles(AggregationEdge.class, pEdge, pStart, pEnd),
-				ClassDiagramEdgeConstraints.noDirectCycles(AssociationEdge.class, pEdge, pStart, pEnd),
-				ClassDiagramEdgeConstraints.noCombinedAssociationAggregation(pEdge, pStart, pEnd)
-		);
+		return constraints;
 	}
 
 	private static boolean validChild(Node pPotentialChild)

diff --git a/src/ca/mcgill/cs/jetuml/diagram/builder/DiagramBuilder.java b/src/ca/mcgill/cs/jetuml/diagram/builder/DiagramBuilder.java
@@ -38,7 +38,6 @@
 import ca.mcgill.cs.jetuml.diagram.Edge;
 import ca.mcgill.cs.jetuml.diagram.Node;
 import ca.mcgill.cs.jetuml.diagram.builder.constraints.ConstraintSet;
-import ca.mcgill.cs.jetuml.diagram.builder.constraints.EdgeConstraints;
 import ca.mcgill.cs.jetuml.diagram.edges.NoteEdge;
 import ca.mcgill.cs.jetuml.diagram.nodes.FieldNode;
 import ca.mcgill.cs.jetuml.diagram.nodes.NoteNode;
@@ -136,12 +135,9 @@ public final boolean canAdd(Edge pEdge, Point pStart, Point pEnd)
 			return false;
 		}
 
-		ConstraintSet constraints = new ConstraintSet(
-				EdgeConstraints.noteEdge(pEdge, startNode.get(), endNode.get()),
-				EdgeConstraints.noteNode(pEdge, startNode.get(), endNode.get())
-		);
-		constraints.merge(getAdditionalEdgeConstraints(pEdge, startNode.get(), endNode.get(), pStart, pEnd));
-		return constraints.satisfied();
+
+		return this.getEdgeConstraints().satisfied(pEdge, startNode.get(), endNode.get(), pStart, pEnd, aDiagram);
+
 	}
 
 	/**
@@ -161,15 +157,9 @@ public boolean canAdd(Node pNode, Point pRequestedPosition)
 	}
 
 	/**
-	 * @param pEdge The edge to add.
-	 * @param pStart The start node.
-	 * @param pEnd The end node.
- 	 * @param pStartPoint the point in the start node.
-	 * @param pEndPoint the point in the end node.
-	 * @return Additional, diagram type-specific constraints for adding edges.
-	 * @pre pEdge != null && pStart != null && pEnd != null && pStartPoint!= null && pEndPoint != null
+	 * @return diagram type-specific constraints for adding edges.
 	 */
-	protected abstract ConstraintSet getAdditionalEdgeConstraints(Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint);
+	protected abstract ConstraintSet getEdgeConstraints();
 
 	/** 
 	 * The default behavior is to position the node so it entirely fits in the diagram, then 

diff --git a/src/ca/mcgill/cs/jetuml/diagram/builder/ObjectDiagramBuilder.java b/src/ca/mcgill/cs/jetuml/diagram/builder/ObjectDiagramBuilder.java
@@ -40,6 +40,15 @@
  */
 public class ObjectDiagramBuilder extends DiagramBuilder
 {
+	private static final ConstraintSet constraints = new ConstraintSet(
+
+			EdgeConstraints.noteEdge(),
+			EdgeConstraints.noteNode(),
+			EdgeConstraints.maxEdges(1),
+			ObjectDiagramEdgeConstraints.collaboration(),
+			ObjectDiagramEdgeConstraints.reference()
+		);
+
 	/**
 	 * Creates a new builder for object diagrams.
 	 * 
@@ -53,13 +62,9 @@ public ObjectDiagramBuilder( Diagram pDiagram )
 	}
 
 	@Override
-	protected ConstraintSet getAdditionalEdgeConstraints(Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint)
+	protected ConstraintSet getEdgeConstraints()
 	{
-		return new ConstraintSet(
-			EdgeConstraints.maxEdges(pEdge, pStart, pEnd, aDiagram, 1),
-			ObjectDiagramEdgeConstraints.collaboration(pEdge, pStart, pEnd),
-			ObjectDiagramEdgeConstraints.reference(pEdge, pStart, pEnd)
-		);
+		return constraints;
 	}
 
 	@Override

diff --git a/src/ca/mcgill/cs/jetuml/diagram/builder/SequenceDiagramBuilder.java b/src/ca/mcgill/cs/jetuml/diagram/builder/SequenceDiagramBuilder.java
@@ -52,6 +52,15 @@ public class SequenceDiagramBuilder extends DiagramBuilder
 {
 	private static final int CALL_NODE_YGAP = 5;
 	private static final ImplicitParameterNodeViewer IMPLICIT_PARAMETER_NODE_VIEWER = new ImplicitParameterNodeViewer();
+	private static final ConstraintSet constraints = new ConstraintSet(
+			EdgeConstraints.noteEdge(),
+			EdgeConstraints.noteNode(),
+			EdgeConstraints.maxEdges(1),
+			SequenceDiagramEdgeConstraints.noEdgesFromParameterTop(),
+			SequenceDiagramEdgeConstraints.returnEdge(),
+			SequenceDiagramEdgeConstraints.singleEntryPoint(),
+			SequenceDiagramEdgeConstraints.callEdgeEnd()
+	);
 
 	/**
 	 * Creates a new builder for sequence diagrams.
@@ -66,16 +75,10 @@ public SequenceDiagramBuilder( Diagram pDiagram )
 	}
 
 	@Override
-	protected ConstraintSet getAdditionalEdgeConstraints(Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint)
+	protected ConstraintSet getEdgeConstraints()
 	{
-		ConstraintSet constraintSet = new ConstraintSet(
-				EdgeConstraints.maxEdges(pEdge, pStart, pEnd, aDiagram, 1),
-				SequenceDiagramEdgeConstraints.noEdgesFromParameterTop(pStart, pStartPoint),
-				SequenceDiagramEdgeConstraints.returnEdge(pEdge, pStart, pEnd, aDiagram),
-				SequenceDiagramEdgeConstraints.singleEntryPoint(pEdge, pStart, aDiagram),
-				SequenceDiagramEdgeConstraints.callEdgeEnd(pEdge, pStart, pEnd, pEndPoint, aDiagram)
-			);
-		return constraintSet;
+
+		return constraints;
 	}
 
 	/**

diff --git a/src/ca/mcgill/cs/jetuml/diagram/builder/StateDiagramBuilder.java b/src/ca/mcgill/cs/jetuml/diagram/builder/StateDiagramBuilder.java
@@ -23,12 +23,10 @@
 
 import ca.mcgill.cs.jetuml.diagram.Diagram;
 import ca.mcgill.cs.jetuml.diagram.DiagramType;
-import ca.mcgill.cs.jetuml.diagram.Edge;
-import ca.mcgill.cs.jetuml.diagram.Node;
 import ca.mcgill.cs.jetuml.diagram.builder.constraints.ConstraintSet;
 import ca.mcgill.cs.jetuml.diagram.builder.constraints.EdgeConstraints;
 import ca.mcgill.cs.jetuml.diagram.builder.constraints.StateDiagramEdgeConstraints;
-import ca.mcgill.cs.jetuml.geom.Point;
+
 
 /**
  * A builder for state diagrams.
@@ -41,19 +39,24 @@ public class StateDiagramBuilder extends DiagramBuilder
 	 * @param pDiagram The diagram to wrap around.
 	 * @pre pDiagram != null;
 	 */
+	private static final ConstraintSet constraints = new ConstraintSet(
+
+			EdgeConstraints.noteEdge(),
+			EdgeConstraints.noteNode(),
+			EdgeConstraints.maxEdges(2),
+			StateDiagramEdgeConstraints.noEdgeFromFinalNode(),
+			StateDiagramEdgeConstraints.noEdgeToInitialNode()
+		);
+
 	public StateDiagramBuilder( Diagram pDiagram )
 	{
 		super( pDiagram );
 		assert pDiagram.getType() == DiagramType.STATE;
 	}
 
 	@Override
-	protected ConstraintSet getAdditionalEdgeConstraints(Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint)
+	protected ConstraintSet getEdgeConstraints()
 	{
-		return new ConstraintSet(
-			EdgeConstraints.maxEdges(pEdge, pStart, pEnd, aDiagram, 2),
-			StateDiagramEdgeConstraints.noEdgeFromFinalNode(pEdge, pStart),
-			StateDiagramEdgeConstraints.noEdgeToInitialNode(pEnd)
-		);
+		return constraints;
 	}
 }
diff --git a/src/ca/mcgill/cs/jetuml/diagram/builder/UseCaseDiagramBuilder.java b/src/ca/mcgill/cs/jetuml/diagram/builder/UseCaseDiagramBuilder.java
@@ -23,17 +23,23 @@
 
 import ca.mcgill.cs.jetuml.diagram.Diagram;
 import ca.mcgill.cs.jetuml.diagram.DiagramType;
-import ca.mcgill.cs.jetuml.diagram.Edge;
-import ca.mcgill.cs.jetuml.diagram.Node;
 import ca.mcgill.cs.jetuml.diagram.builder.constraints.ConstraintSet;
 import ca.mcgill.cs.jetuml.diagram.builder.constraints.EdgeConstraints;
-import ca.mcgill.cs.jetuml.geom.Point;
+
 
 /**
  * A builder for use case diagram.
  */
 public class UseCaseDiagramBuilder extends DiagramBuilder
 {
+	private static final ConstraintSet constraints = new ConstraintSet(
+
+			EdgeConstraints.noteEdge(),
+			EdgeConstraints.noteNode(),
+			EdgeConstraints.maxEdges(1),
+			EdgeConstraints.noSelfEdge()
+	);
+
 	/**
 	 * Creates a new builder for use case diagrams.
 	 * 
@@ -47,11 +53,8 @@ public UseCaseDiagramBuilder( Diagram pDiagram )
 	}
 
 	@Override
-	protected ConstraintSet getAdditionalEdgeConstraints(Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint)
+	protected ConstraintSet getEdgeConstraints()
 	{
-		return new ConstraintSet(
-				EdgeConstraints.maxEdges(pEdge, pStart, pEnd, aDiagram, 1),
-				EdgeConstraints.noSelfEdge(pStart, pEnd)
-		);
+		return constraints;
 	}
 }
diff --git a/src/ca/mcgill/cs/jetuml/diagram/builder/constraints/ClassDiagramEdgeConstraints.java b/src/ca/mcgill/cs/jetuml/diagram/builder/constraints/ClassDiagramEdgeConstraints.java
@@ -21,27 +21,37 @@
 
 package ca.mcgill.cs.jetuml.diagram.builder.constraints;
 
+import ca.mcgill.cs.jetuml.diagram.Diagram;
 import ca.mcgill.cs.jetuml.diagram.Edge;
 import ca.mcgill.cs.jetuml.diagram.Node;
 import ca.mcgill.cs.jetuml.diagram.edges.AggregationEdge;
 import ca.mcgill.cs.jetuml.diagram.edges.AssociationEdge;
 import ca.mcgill.cs.jetuml.diagram.edges.DependencyEdge;
 import ca.mcgill.cs.jetuml.diagram.edges.GeneralizationEdge;
+import ca.mcgill.cs.jetuml.geom.Point;
 
 /**
  * Methods to create edge addition constraints that only apply to
  * class diagrams. CSOFF:
  */
 public final class ClassDiagramEdgeConstraints
 {
-	private ClassDiagramEdgeConstraints() {}
+	private ClassDiagramEdgeConstraints() {
+
+		noSelfGeneralization();
+		noSelfDependency();
+		noDirectCycles(GeneralizationEdge.class);
+		noDirectCycles(AggregationEdge.class);
+		noDirectCycles(AssociationEdge.class);
+		noCombinedAssociationAggregation();
+	}
 
 	/*
 	 * Self edges are not allowed for Generalization edges.
 	 */
-	public static Constraint noSelfGeneralization(Edge pEdge, Node pStart, Node pEnd)
+	public static Constraint noSelfGeneralization()
 	{
-		return ()-> 
+		return (Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint, Diagram pDiagram)-> 
 		{
 			return !( pEdge.getClass() == GeneralizationEdge.class && pStart == pEnd );
 		};
@@ -50,9 +60,9 @@ public static Constraint noSelfGeneralization(Edge pEdge, Node pStart, Node pEnd
 	/*
 	 * Self edges are not allowed for Dependency edges.
 	 */
-	public static Constraint noSelfDependency(Edge pEdge, Node pStart, Node pEnd)
+	public static Constraint noSelfDependency()
 	{
-		return () ->
+		return (Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint, Diagram pDiagram) ->
 		{
 			return !( pEdge.getClass() == DependencyEdge.class && pStart == pEnd );
 		};
@@ -61,10 +71,9 @@ public static Constraint noSelfDependency(Edge pEdge, Node pStart, Node pEnd)
 	/*
 	 * There can't be two edges of a given type, one in each direction, between two nodes.
 	 */
-	public static Constraint noDirectCycles(Class<? extends Edge> pEdgeType, 
-			Edge pEdge, Node pStart, Node pEnd)
+	public static Constraint noDirectCycles(Class<? extends Edge> pEdgeType)
 	{
-		return () ->
+		return (Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint, Diagram pDiagram) ->
 		{
 			if( pEdge.getClass() != pEdgeType )
 			{
@@ -84,9 +93,9 @@ public static Constraint noDirectCycles(Class<? extends Edge> pEdgeType,
 	/*
 	 * There can't be both an association and an aggregation edge between two nodes
 	 */
-	public static Constraint noCombinedAssociationAggregation(Edge pEdge, Node pStart, Node pEnd)
+	public static Constraint noCombinedAssociationAggregation()
 	{
-		return () ->
+		return (Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint, Diagram pDiagram) ->
 		{
 			if( pEdge.getClass() != AssociationEdge.class && pEdge.getClass() != AggregationEdge.class )
 			{
@@ -105,4 +114,4 @@ public static Constraint noCombinedAssociationAggregation(Edge pEdge, Node pStar
 			return true;
 		};
 	}
-}
+}
diff --git a/src/ca/mcgill/cs/jetuml/diagram/builder/constraints/Constraint.java b/src/ca/mcgill/cs/jetuml/diagram/builder/constraints/Constraint.java
@@ -21,6 +21,11 @@
 
 package ca.mcgill.cs.jetuml.diagram.builder.constraints;
 
+import ca.mcgill.cs.jetuml.diagram.Diagram;
+import ca.mcgill.cs.jetuml.diagram.Edge;
+import ca.mcgill.cs.jetuml.diagram.Node;
+import ca.mcgill.cs.jetuml.geom.Point;
+
 /**
  * Represents a generic constraint.
  */
@@ -30,5 +35,5 @@ public interface Constraint
 	 * Indicates if the constraint is satisfied.
 	 * @return True if this constraint is satisfied.
 	 */
-	boolean satisfied();
-}
+	boolean satisfied(Edge pEdge, Node pStart, Node pEnd, Point pStartPoint, Point pEndPoint, Diagram pDiagram);
+}