RED

Issue-Ref: 4181
Issue-Url: af3#4181



Signed-off-by: Simon Barner <barner@fortiss.org>

org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/lwfxef/.ratings

 DiagramCoordinate.java 6b00aec99054d4cd19003a72bd4e5e774ac6a641 GREEN
 DiagramLayers.java aa1f95dbae290c8b00202abe4385b01b8f36e5ab GREEN
-DiagramViewer.java 5359e94ca4a4ca60be60ecb9faaa4f8e1ce8d48d YELLOW
+DiagramViewer.java 504ae4708be551d72bf4dee5ef9fbfbdbadef18f RED
 DiagramViewerDefaultTags.java 6230763252409c60009ab8887b4ef582cf883229 GREEN
 DiagramViewerFeatures.java 3dd78d9c117fc156924a151c6f8d770c53c103bc GREEN
 DiagramViewerSelection.java e833f592543bc97077907d980a39b123fc4044e6 GREEN
 EDragGesture.java 5cfa098d3877db11981c2750e5e103156d62fc5e GREEN
 FeedbackChange.java b088fa89af648f1674f2f9c1f7f99d585ce801ca GREEN
-GridCanvasVisual.java 354839685eca8f9f2dd464d5713feb848f868fb1 YELLOW
+GridCanvasVisual.java d8499f9458032ececff706c170d5f747daf20050 RED
 MVCBundleManager.java 18667b4ed98da124b7c1bc7a103e95232df9ad49 GREEN
 MouseState.java 3d9993f799d5d74bc74ac03b46e4a1857c4d267e GREEN
 SVGExporter.java cbbd1eceb2910fd5c1693e05c5303a193127b9db GREEN

org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/lwfxef/DiagramViewer.java

@@ -177,17 +177,20 @@ public class DiagramViewer {
 		setHgrow(viewerPane, ALWAYS);
 		setVgrow(viewerPane, ALWAYS);
 
+		// TODO factorize to method createScrollBar(javafx.geometry.Orientation)
 		// vertical scrollbar
 		verticalScrollbar = new ScrollBar();
 		verticalScrollbar.setOrientation(Orientation.VERTICAL);
 		scrolledPane.add(verticalScrollbar, 2, 1);
 		verticalScrollbar.valueProperty().addListener((obs, ov, nv) -> {
 			hideContextMenu();
+			// TODO (SB): Use "speaking" variable name
 			Point2D p1 = layers.parentToLocal(0, 0);
 			transform.appendTranslation(0, -nv.doubleValue() + p1.getY());
 			viewerManager.gridCanvasVisual.updateNodes(layers);
 		});
 
+		// TODO Use createScrollbar(javafx.geometry.Orientation)
 		// horizontal scrollbar
 		horizontalScrollbar = new ScrollBar();
 		scrolledPane.add(horizontalScrollbar, 1, 2);
@@ -283,6 +286,7 @@ public class DiagramViewer {
 		return exporter.export();
 	}
 
+	// TOOD (SB): Factorize commonalities with updateHorizontalScrollbar()
 	/** Updates the vertical scrollbar. */
 	private void updateVerticalScrollbar() {
 		// The size of the content (in diagram coordinates)
@@ -303,6 +307,8 @@ public class DiagramViewer {
 		// visibleHeight is visible. Since the scrollbar
 		// goes from 0 to contentHeight, this means we need:
 		// contentHeight / visibleAmount == (contentHeight + visibleHeight) / visibleHeight
+		// TODO (SB): create local variable visibleAmount (and use it later for
+		// verticalScrollbar.setBlockIncrement)
 		verticalScrollbar
 				.setVisibleAmount(contentHeight * visibleHeight / (contentHeight + visibleHeight));
 
@@ -442,9 +448,7 @@ public class DiagramViewer {
 		return viewerManager.modelFactory;
 	}
 
-	/**
-	 * Clamp content so that it can't be moved beyond the borders.
-	 */
+	/** Clamp content so that it can't be moved beyond the borders. */
 	private void enforceBounds() {
 		// The size of the content (in diagram coordinates)
 		Bounds contentBounds = layers.getBoundsInLocal();
@@ -453,10 +457,10 @@ public class DiagramViewer {
 				new BoundingBox(0, 0, contentBounds.getMaxX(), contentBounds.getMaxY());
 		Bounds contentInParent = layers.localToParent(enforceContentBounds);
 
-		double leftBoundCorrection = Math.max(contentInParent.getMinX(), 0);
-		double topBoundCorrection = Math.max(contentInParent.getMinY(), 0);
-		double rightBoundCorrection = Math.min(contentInParent.getMaxX(), 0);
-		double bottomBoundCorrection = Math.min(contentInParent.getMaxY(), 0);
+		double leftBoundCorrection = max(contentInParent.getMinX(), 0);
+		double topBoundCorrection = max(contentInParent.getMinY(), 0);
+		double rightBoundCorrection = min(contentInParent.getMaxX(), 0);
+		double bottomBoundCorrection = min(contentInParent.getMaxY(), 0);
 		Point2D clampedInParent = new Point2D(leftBoundCorrection + rightBoundCorrection,
 				topBoundCorrection + bottomBoundCorrection);
 		Point2D correctionLocal = layers.parentToLocal(clampedInParent);
@@ -465,9 +469,7 @@ public class DiagramViewer {
 				originLocal.getY() - correctionLocal.getY());
 	}
 
-	/**
-	 * Apply a transformation to the content.
-	 */
+	/** Apply a transformation to the content. */
 	void appendContentTransform(Transform t) {
 		transform.append(t);
 		enforceBounds();
@@ -497,19 +499,19 @@ public class DiagramViewer {
 				double hmax = horizontalScrollbar.getMax();
 				double hmin = horizontalScrollbar.getMin();
 				double nval = horizontalScrollbar.getValue() -
-						evt.getDeltaY() * Math.abs(p1.getX() - p0.getX());
+						evt.getDeltaY() * abs(p1.getX() - p0.getX());
 				horizontalScrollbar.setValue(max(hmin, min(hmax, nval)));
 			} else {
 				Point2D p0 = layers.screenToLocal(0, 0);
 				Point2D p1 = layers.screenToLocal(1, 1);
 				double vmax = verticalScrollbar.getMax();
 				double vmin = verticalScrollbar.getMin();
-				double vval = verticalScrollbar.getValue() -
-						evt.getDeltaY() * Math.abs(p1.getY() - p0.getY());
+				double vval =
+						verticalScrollbar.getValue() - evt.getDeltaY() * abs(p1.getY() - p0.getY());
 				double hmax = horizontalScrollbar.getMax();
 				double hmin = horizontalScrollbar.getMin();
 				double hval = horizontalScrollbar.getValue() -
-						evt.getDeltaX() * Math.abs(p1.getX() - p0.getX());
+						evt.getDeltaX() * abs(p1.getX() - p0.getX());
 				verticalScrollbar.setValue(max(vmin, min(vmax, vval)));
 				horizontalScrollbar.setValue(max(hmin, min(hmax, hval)));
 			}
@@ -585,9 +587,7 @@ public class DiagramViewer {
 		setSingleSelectedMVCBundle(null);
 	}
 
-	/**
-	 * Converts the given absolute diagram coordinates to node-local coordinates.
-	 */
+	/** Converts the given absolute diagram coordinates to node-local coordinates. */
 	/* package */ DiagramCoordinate convertEventToDiagramCoordinates(Point2D absoluteLocation,
 			Node node) {
 		return convertEventToDiagramCoordinates(absoluteLocation.getX(), absoluteLocation.getY(),
@@ -662,10 +662,7 @@ public class DiagramViewer {
 		}
 	}
 
-	/**
-	 * Starts the feedback of the link creation line and shows the possible target
-	 * feedbacks.
-	 */
+	/** Starts the feedback of the link creation line and shows the possible target feedbacks. */
 	public void startNewLinkLineFeedback(IMVCBundle linkStartBundle, Node node,
 			Point2D locationInDiagram) {
 		if(node == null || locationInDiagram == null) {
@@ -898,13 +895,9 @@ public class DiagramViewer {
 		}
 	}
 
-	/**
-	 * Mouse controller for moving the content using control-drag.
-	 */
+	/** Mouse controller for moving the content using control-drag. */
 	private final class ContentDragController {
-		/**
-		 * Last point within a control-drag gesture.
-		 */
+		/** Last point within a control-drag gesture. */
 		private Point2D dragPoint;
 		/**
 		 * Has the mouse button been released within a control-drag gesture? If so, the next click
@@ -912,9 +905,7 @@ public class DiagramViewer {
 		 */
 		private boolean buttonReleased;
 
-		/**
-		 * Register the event filters of this controller.
-		 */
+		/** Register the event filters of this controller. */
 		/* package */ void registerEventFilters(Node node) {
 			node.addEventFilter(MouseEvent.MOUSE_DRAGGED, dragDetected);
 			node.addEventFilter(MouseEvent.ANY, dragged);
@@ -922,9 +913,7 @@ public class DiagramViewer {
 			node.addEventFilter(MouseEvent.MOUSE_CLICKED, mouseClicked);
 		}
 
-		/**
-		 * Control-drag gesture starts when drag is detected.
-		 */
+		/** Control-drag gesture starts when drag is detected. */
 		private final EventHandler<MouseEvent> dragDetected = evt -> {
 			if(dragPoint == null && evt.isControlDown() && evt.getButton() == MouseButton.PRIMARY) {
 				dragPoint = getLayers().sceneToLocal(evt.getSceneX(), evt.getSceneY());
@@ -933,9 +922,7 @@ public class DiagramViewer {
 			}
 		};
 
-		/**
-		 * While dragging, the content is translated appropriately.
-		 */
+		/** While dragging, the content is translated appropriately. */
 		private final EventHandler<MouseEvent> dragged = evt -> {
 			if(dragPoint != null) {
 				evt.consume();
@@ -950,9 +937,7 @@ public class DiagramViewer {
 			}
 		};
 
-		/**
-		 * If the mouse button is released, the guesture should be stopped.
-		 */
+		/** If the mouse button is released, the guesture should be stopped. */
 		private final EventHandler<MouseEvent> mouseReleased = evt -> {
 			if(dragPoint != null && evt.getButton() == MouseButton.PRIMARY) {
 				dragPoint = null;
@@ -961,9 +946,7 @@ public class DiagramViewer {
 			}
 		};
 
-		/**
-		 * The first click event after the released event should be filtered.
-		 */
+		/** The first click event after the released event should be filtered. */
 		private final EventHandler<MouseEvent> mouseClicked = evt -> {
 			if(buttonReleased && evt.getButton() == MouseButton.PRIMARY) {
 				evt.consume();

org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/lwfxef/GridCanvasVisual.java

@@ -9,6 +9,9 @@
  *******************************************************************************/
 package org.fortiss.tooling.common.ui.javafx.lwfxef;
 
+import static java.lang.Double.doubleToLongBits;
+import static java.lang.Math.abs;
+
 import java.util.Objects;
 
 import org.fortiss.tooling.common.ui.javafx.lwfxef.DiagramViewerFeatures.IndicatorType;
@@ -52,19 +55,13 @@ final class GridCanvasVisual implements IVisual {
 	 */
 	private final Rectangle leftOuterBorder;
 
-	/**
-	 * A rectangle for the top outer border.
-	 */
+	/** A rectangle for the top outer border. */
 	private final Rectangle topOuterBorder;
 
-	/**
-	 * Width of the whole shown grid.
-	 */
+	/** Width of the whole shown grid. */
 	double width;
 
-	/**
-	 * Height of the whole shown grid.
-	 */
+	/** Height of the whole shown grid. */
 	double height;
 
 	/** Constructor. */
@@ -145,38 +142,29 @@ final class GridCanvasVisual implements IVisual {
 		pane.requestFocus();
 	}
 
-	/**
-	 * Compute zoom factor from current content transformation.
-	 * 
-	 * @return zoom factor for grid
-	 */
+	/** Returns the zoom factor from current content transformation. */
 	private double getZoomFactor() {
 		Point2D p0 = viewer.getLayers().sceneToLocal(pane.localToScene(0, 0));
 		Point2D p1 = viewer.getLayers().sceneToLocal(pane.localToScene(1, 1));
-		return 1 / Math.abs(p1.getX() - p0.getX());
+
+		// TODO (SB): Add check against division by zero. (assert() if we are sure that it will not
+		// happen, otherwise maybe throw a RuntimeException)
+		return 1 / abs(p1.getX() - p0.getX());
 	}
 
-	/**
-	 * Structure to represent the current location of the grid.
-	 */
+	/** Structure to represent the current location of the grid. */
 	private static class GridOffset {
-		/**
-		 * Grid x coordinate of upper left corner
-		 */
+		/** Grid x coordinate of upper left corner. */
 		int gridX;
-		/**
-		 * Grid y coordinate of upper left corner
-		 */
+
+		/** Grid y coordinate of upper left corner. */
 		int gridY;
-		/**
-		 * Insets in the grid square between (gridX, gridY) and (gridX + 1, gridY +1)
-		 */
+
+		/** Insets in the grid square between (gridX, gridY) and (gridX + 1, gridY +1). */
 		Point2D insets;
 	}
 
-	/**
-	 * Compute the grid offset current content transformation.
-	 */
+	/** Compute the grid offset current content transformation. */
 	private GridOffset getGridOffset() {
 		DiagramViewerFeatures features = viewer.getFeatures();
 		Point2D p0 = viewer.getLayers().sceneToLocal(pane.localToScene(0, 0));
@@ -195,33 +183,25 @@ final class GridCanvasVisual implements IVisual {
 	 */
 	private static class GridParameters {
 
-		/**
-		 * Horizontal grid spacing
-		 */
+		/** Horizontal grid spacing. */
 		private double hSpacing;
-		/**
-		 * Vertical grid spacing
-		 */
+
+		/** Vertical grid spacing. */
 		private double vSpacing;
-		/**
-		 * Zoom factor
-		 */
+
+		/** Zoom factor. */
 		private double zf;
-		/**
-		 * Grid indicator size
-		 */
+
+		/** Grid indicator size. */
 		private double indicatorSize;
-		/**
-		 * Grid indicator type
-		 */
+
+		/** Grid indicator type. */
 		private IndicatorType indicatorType;
-		/**
-		 * Grid background color
-		 */
+
+		/** Grid background color. */
 		private Color backgroundColor;
-		/**
-		 * Grid indicator color
-		 */
+
+		/** Grid indicator color. */
 		private Color indicatorColor;
 
 		/** {@inheritDoc} */
@@ -234,27 +214,31 @@ final class GridCanvasVisual implements IVisual {
 		/** {@inheritDoc} */
 		@Override
 		public boolean equals(Object obj) {
-			if(this == obj)
+			if(this == obj) {
 				return true;
-			if(obj == null)
+			}
+			if(obj == null) {
 				return false;
-			if(getClass() != obj.getClass())
+			}
+			if(getClass() != obj.getClass()) {
 				return false;
+			}
 			GridParameters other = (GridParameters)obj;
+
+			// TODO Add comment to explain the condition. In particular, the use of
+			// doubleToLongBits() is not obvious. Also, splitting the expression into several
+			// variables (with self-explanatory names) might improve understandability
 			return Objects.equals(backgroundColor, other.backgroundColor) &&
-					Double.doubleToLongBits(hSpacing) == Double.doubleToLongBits(other.hSpacing) &&
+					doubleToLongBits(hSpacing) == doubleToLongBits(other.hSpacing) &&
 					Objects.equals(indicatorColor, other.indicatorColor) &&
-					Double.doubleToLongBits(indicatorSize) == Double
-							.doubleToLongBits(other.indicatorSize) &&
+					doubleToLongBits(indicatorSize) == doubleToLongBits(other.indicatorSize) &&
 					indicatorType == other.indicatorType &&
-					Double.doubleToLongBits(vSpacing) == Double.doubleToLongBits(other.vSpacing) &&
-					Math.abs(zf - other.zf) < 0.00001;
+					doubleToLongBits(vSpacing) == doubleToLongBits(other.vSpacing) &&
+					abs(zf - other.zf) < 0.00001;
 		}
 	}
 
-	/**
-	 * Update the pane node
-	 */
+	/** Update the pane node. */
 	private void updateContent() {
 		DiagramViewerFeatures features = viewer.getFeatures();
 
@@ -291,14 +275,10 @@ final class GridCanvasVisual implements IVisual {
 		}
 	}
 
-	/**
-	 * The parameters of what is currently drawn on the gridCanvas.
-	 */
+	/** The parameters of what is currently drawn on the gridCanvas. */
 	private GridParameters currentGridCanvasParameters = null;
 
-	/**
-	 * Draw gridCanvas with the given parameters.
-	 */
+	/** Draw gridCanvas with the given parameters. */
 	private void paintCanvas(GridParameters parameters) {
 		double targetCanvasWidth = width + parameters.zf * parameters.hSpacing;
 		double targetCanvasHeight = height + parameters.zf * parameters.vSpacing;