From 764152f168a501dbfc9f19e9002b5b37763f5209 Mon Sep 17 00:00:00 2001
From: Hallvard Traetteberg <hal@ntnu.no>
Date: Tue, 20 Aug 2019 20:16:31 +0200
Subject: [PATCH] Fikset en del av det som ble rapportert av PMD og SpotBugs
---
.../doc/AbstractDocumentStorageImpl.java | 2 +-
.../java/fxutil/doc/FileMenuController.java | 11 ++---
.../fxutil/doc/SimpleJsonFileStorageImpl.java | 5 ++-
.../src/main/java/simpleex/core/LatLong.java | 16 ++++---
.../src/main/java/simpleex/core/LatLongs.java | 6 +--
.../simpleex/json/LatLongDeserializer.java | 4 +-
.../java/simpleex/json/LatLongSerializer.java | 4 +-
.../simpleex/ui/DraggableNodeController.java | 42 ++++++++++---------
.../java/simpleex/ui/FxAppController.java | 26 ++++++------
.../src/main/java/simpleex/ui/MapMarker.java | 4 +-
.../test/java/simpleex/core/LatLongTest.java | 4 +-
.../test/java/simpleex/core/LatLongsTest.java | 4 +-
.../java/simpleex/json/AbstractJsonTest.java | 2 +-
.../java/simpleex/json/LatLongJsonTest.java | 6 +--
.../src/test/java/simpleex/ui/FxAppTest.java | 6 +--
15 files changed, 75 insertions(+), 67 deletions(-)
diff --git a/simpleexample/src/main/java/fxutil/doc/AbstractDocumentStorageImpl.java b/simpleexample/src/main/java/fxutil/doc/AbstractDocumentStorageImpl.java
index 13bc9e9..d602b6c 100644
--- a/simpleexample/src/main/java/fxutil/doc/AbstractDocumentStorageImpl.java
+++ b/simpleexample/src/main/java/fxutil/doc/AbstractDocumentStorageImpl.java
@@ -20,7 +20,7 @@ import java.util.Collection;
*/
public abstract class AbstractDocumentStorageImpl<D, L> implements IDocumentStorage<L>, IDocumentPersistence<D, L> {
- private L documentLocation;
+ private L documentLocation = null;
@Override
public L getDocumentLocation() {
diff --git a/simpleexample/src/main/java/fxutil/doc/FileMenuController.java b/simpleexample/src/main/java/fxutil/doc/FileMenuController.java
index 3aef54e..406a211 100644
--- a/simpleexample/src/main/java/fxutil/doc/FileMenuController.java
+++ b/simpleexample/src/main/java/fxutil/doc/FileMenuController.java
@@ -186,18 +186,15 @@ public class FileMenuController {
inputDialog.setHeaderText("Enter URL to import from");
inputDialog.setContentText("Enter URL: ");
// https://developer.garmin.com/downloads/connect-api/sample_file.gpx
- URL url = null;
- while (url == null) {
+ while (true) {
final Optional<String> result = inputDialog.showAndWait();
if (! result.isPresent()) {
- return;
+ break;
}
try {
- url = new URL(result.get());
- if (handleURLImportAction(url)) {
- return;
+ if (handleURLImportAction(new URL(result.get()))) {
+ break;
}
- url = null;
inputDialog.setHeaderText("Problems reading it...");
inputDialog.setContentText("Enter another URL: ");
} catch (final MalformedURLException e1) {
diff --git a/simpleexample/src/main/java/fxutil/doc/SimpleJsonFileStorageImpl.java b/simpleexample/src/main/java/fxutil/doc/SimpleJsonFileStorageImpl.java
index cf35d0f..1ad4d73 100644
--- a/simpleexample/src/main/java/fxutil/doc/SimpleJsonFileStorageImpl.java
+++ b/simpleexample/src/main/java/fxutil/doc/SimpleJsonFileStorageImpl.java
@@ -3,6 +3,7 @@ package fxutil.doc;
import java.io.File;
import java.io.FileOutputStream;
import java.io.InputStream;
+import java.io.OutputStream;
import java.util.Collection;
import java.util.Collections;
@@ -34,7 +35,9 @@ public abstract class SimpleJsonFileStorageImpl<T> extends AbstractDocumentStora
@Override
public void saveDocument(final T document, final File documentLocation) throws Exception {
- objectMapper.writeValue(new FileOutputStream(documentLocation, false), document);
+ try (OutputStream outputStream = new FileOutputStream(documentLocation, false)) {
+ objectMapper.writeValue(outputStream, document);
+ }
}
@Override
diff --git a/simpleexample/src/main/java/simpleex/core/LatLong.java b/simpleexample/src/main/java/simpleex/core/LatLong.java
index 74fa119..53ea3af 100644
--- a/simpleexample/src/main/java/simpleex/core/LatLong.java
+++ b/simpleexample/src/main/java/simpleex/core/LatLong.java
@@ -2,7 +2,7 @@ package simpleex.core;
public class LatLong {
- public final double latitude, longitude;
+ private final double latitude, longitude;
public LatLong(final double latitude, final double longitude) {
super();
@@ -12,6 +12,14 @@ public class LatLong {
public final static String SEPARATOR = ",";
+ public double getLatitude() {
+ return latitude;
+ }
+
+ public double getLongitude() {
+ return longitude;
+ }
+
@Override
public String toString() {
return latitude + SEPARATOR + longitude;
@@ -20,10 +28,8 @@ public class LatLong {
@Override
public int hashCode() {
final int prime = 31;
- int result = 1;
- long temp;
- temp = Double.doubleToLongBits(latitude);
- result = prime * result + (int) (temp ^ (temp >>> 32));
+ long temp = Double.doubleToLongBits(latitude);
+ int result = prime + (int) (temp ^ (temp >>> 32));
temp = Double.doubleToLongBits(longitude);
result = prime * result + (int) (temp ^ (temp >>> 32));
return result;
diff --git a/simpleexample/src/main/java/simpleex/core/LatLongs.java b/simpleexample/src/main/java/simpleex/core/LatLongs.java
index 98f3837..1115017 100644
--- a/simpleexample/src/main/java/simpleex/core/LatLongs.java
+++ b/simpleexample/src/main/java/simpleex/core/LatLongs.java
@@ -47,17 +47,17 @@ public class LatLongs implements Iterable<LatLong> {
return pos;
}
- public int addLatLongs(final Collection<LatLong> latLongs) {
+ public final int addLatLongs(final Collection<LatLong> latLongs) {
final int pos = this.latLongs.size();
this.latLongs.addAll(latLongs);
return pos;
}
- public int addLatLongs(final LatLong... latLongs) {
+ public final int addLatLongs(final LatLong... latLongs) {
return addLatLongs(List.of(latLongs));
}
- public int addLatLongs(final double... latLongsArray) {
+ public final int addLatLongs(final double... latLongsArray) {
final Collection<LatLong> latLongs = new ArrayList<>(latLongsArray.length / 2);
for (int i = 0; i < latLongsArray.length; i += 2) {
latLongs.add(new LatLong(latLongsArray[i], latLongsArray[i + 1]));
diff --git a/simpleexample/src/main/java/simpleex/json/LatLongDeserializer.java b/simpleexample/src/main/java/simpleex/json/LatLongDeserializer.java
index 9851952..e78c577 100644
--- a/simpleexample/src/main/java/simpleex/json/LatLongDeserializer.java
+++ b/simpleexample/src/main/java/simpleex/json/LatLongDeserializer.java
@@ -14,6 +14,8 @@ import simpleex.core.LatLong;
public class LatLongDeserializer extends JsonDeserializer<LatLong> {
+ private static final int ARRAY_JSON_NODE_SIZE = 2;
+
@Override
public LatLong deserialize(final JsonParser jsonParser, final DeserializationContext deserContext) throws IOException, JsonProcessingException {
final JsonNode jsonNode = jsonParser.getCodec().readTree(jsonParser);
@@ -28,7 +30,7 @@ public class LatLongDeserializer extends JsonDeserializer<LatLong> {
return new LatLong(latitude, longitude);
} else if (jsonNode instanceof ArrayNode) {
final ArrayNode locationArray = (ArrayNode) jsonNode;
- if (locationArray.size() == 2) {
+ if (locationArray.size() == ARRAY_JSON_NODE_SIZE) {
final double latitude = locationArray.get(0).asDouble();
final double longitude = locationArray.get(1).asDouble();
return new LatLong(latitude, longitude);
diff --git a/simpleexample/src/main/java/simpleex/json/LatLongSerializer.java b/simpleexample/src/main/java/simpleex/json/LatLongSerializer.java
index 5c93960..0a12779 100644
--- a/simpleexample/src/main/java/simpleex/json/LatLongSerializer.java
+++ b/simpleexample/src/main/java/simpleex/json/LatLongSerializer.java
@@ -17,9 +17,9 @@ public class LatLongSerializer extends JsonSerializer<LatLong> {
public void serialize(final LatLong latLon, final JsonGenerator jsonGen, final SerializerProvider provider) throws IOException {
jsonGen.writeStartObject();
jsonGen.writeFieldName(LATITUDE_FIELD_NAME);
- jsonGen.writeNumber(latLon.latitude);
+ jsonGen.writeNumber(latLon.getLatitude());
jsonGen.writeFieldName(LONGITUDE_FIELD_NAME);
- jsonGen.writeNumber(latLon.longitude);
+ jsonGen.writeNumber(latLon.getLongitude());
jsonGen.writeEndObject();
}
}
diff --git a/simpleexample/src/main/java/simpleex/ui/DraggableNodeController.java b/simpleexample/src/main/java/simpleex/ui/DraggableNodeController.java
index 4be472f..36dab05 100644
--- a/simpleexample/src/main/java/simpleex/ui/DraggableNodeController.java
+++ b/simpleexample/src/main/java/simpleex/ui/DraggableNodeController.java
@@ -1,5 +1,7 @@
package simpleex.ui;
+import java.util.Optional;
+
import javafx.event.EventHandler;
import javafx.geometry.Point2D;
import javafx.scene.Node;
@@ -10,13 +12,13 @@ public class DraggableNodeController {
public DraggableNodeController() {
}
- public DraggableNodeController(final NodeDraggedHandler nodeDraggedHandler) {
+ public DraggableNodeController(final Optional<NodeDraggedHandler> nodeDraggedHandler) {
setNodeDraggedHandler(nodeDraggedHandler);
}
- private NodeDraggedHandler nodeDraggedHandler;
+ private Optional<NodeDraggedHandler> nodeDraggedHandler = Optional.empty();
- public void setNodeDraggedHandler(final NodeDraggedHandler nodeDraggedHandler) {
+ public final void setNodeDraggedHandler(final Optional<NodeDraggedHandler> nodeDraggedHandler) {
this.nodeDraggedHandler = nodeDraggedHandler;
}
@@ -26,7 +28,7 @@ public class DraggableNodeController {
this.immediate = immediate;
}
- private Node currentNode = null;
+ private Optional<Node> currentNode = Optional.empty();
private Point2D startPoint = null;
private Point2D startTranslate = null;
@@ -47,16 +49,16 @@ public class DraggableNodeController {
}
private void mousePressed(final MouseEvent mouseEvent) {
- if (currentNode == null && mouseEvent.getSource() instanceof Node) {
- currentNode = (Node) mouseEvent.getSource();
+ if (currentNode.isEmpty() && mouseEvent.getSource() instanceof Node) {
+ currentNode = Optional.of((Node) mouseEvent.getSource());
startPoint = new Point2D(mouseEvent.getSceneX(), mouseEvent.getSceneY());
- startTranslate = new Point2D(currentNode.getTranslateX(), currentNode.getTranslateY());
+ startTranslate = new Point2D(currentNode.get().getTranslateX(), currentNode.get().getTranslateY());
mouseEvent.consume();
}
}
private void mouseDragged(final MouseEvent mouseEvent) {
- if (currentNode != null && currentNode == mouseEvent.getSource()) {
+ if (currentNode.isPresent() && currentNode == mouseEvent.getSource()) {
final double dx = mouseEvent.getSceneX() - startPoint.getX();
final double dy = mouseEvent.getSceneY() - startPoint.getY();
updateNode(dx, dy);
@@ -64,27 +66,27 @@ public class DraggableNodeController {
}
protected void updateNode(final double dx, final double dy) {
- if (immediate && nodeDraggedHandler != null) {
- nodeDraggedHandler.nodeDragged(currentNode, dx, dy);
+ if (immediate && nodeDraggedHandler.isPresent()) {
+ nodeDraggedHandler.get().nodeDragged(currentNode.get(), dx, dy);
startPoint = startPoint.add(dx, dy);
- } else {
- currentNode.setTranslateX(startTranslate.getX() + dx);
- currentNode.setTranslateY(startTranslate.getY() + dy);
+ } else if (currentNode.isPresent()) {
+ currentNode.get().setTranslateX(startTranslate.getX() + dx);
+ currentNode.get().setTranslateY(startTranslate.getY() + dy);
}
}
private void mouseReleased(final MouseEvent mouseEvent) {
- if (currentNode != null && currentNode == mouseEvent.getSource()) {
+ if (currentNode.isPresent() && currentNode.get() == mouseEvent.getSource()) {
final double dx = mouseEvent.getSceneX() - startPoint.getX();
final double dy = mouseEvent.getSceneY() - startPoint.getY();
if (! immediate) {
- currentNode.setTranslateX(startTranslate.getX());
- currentNode.setTranslateY(startTranslate.getY());
+ currentNode.get().setTranslateX(startTranslate.getX());
+ currentNode.get().setTranslateY(startTranslate.getY());
}
- final Node node = currentNode;
- currentNode = null;
- if (nodeDraggedHandler != null) {
- nodeDraggedHandler.nodeDragged(node, dx, dy);
+ final Node node = currentNode.get();
+ currentNode = Optional.empty();
+ if (nodeDraggedHandler.isPresent()) {
+ nodeDraggedHandler.get().nodeDragged(node, dx, dy);
}
}
}
diff --git a/simpleexample/src/main/java/simpleex/ui/FxAppController.java b/simpleexample/src/main/java/simpleex/ui/FxAppController.java
index ab1bc32..b8968b6 100644
--- a/simpleexample/src/main/java/simpleex/ui/FxAppController.java
+++ b/simpleexample/src/main/java/simpleex/ui/FxAppController.java
@@ -1,6 +1,7 @@
package simpleex.ui;
import java.io.File;
+import java.util.Optional;
import fxmapcontrol.Location;
import fxmapcontrol.MapBase;
@@ -47,7 +48,7 @@ public class FxAppController extends FileMenuController implements IDocumentList
private MapBase mapView;
private MapItemsControl<MapNode> markersParent;
- private MapMarker marker = null;
+ private Optional<MapMarker> marker = Optional.empty();
private DraggableNodeController draggableMapController = null;
private DraggableNodeController draggableMarkerController = null;
@@ -64,10 +65,10 @@ public class FxAppController extends FileMenuController implements IDocumentList
zoomSlider.setValue(8);
markersParent = new MapItemsControl<MapNode>();
mapView.getChildren().add(markersParent);
- draggableMapController = new DraggableNodeController(this::handleMapDragged);
+ draggableMapController = new DraggableNodeController(Optional.of(this::handleMapDragged));
draggableMapController.setImmediate(true);
draggableMapController.attach(mapView);
- draggableMarkerController = new DraggableNodeController(this::handleMarkerDragged);
+ draggableMarkerController = new DraggableNodeController(Optional.of(this::handleMarkerDragged));
// the location list
locationListView.getSelectionModel().selectedIndexProperty().addListener((prop, oldValue, newValue) -> updateMapMarker(true));
}
@@ -81,7 +82,7 @@ public class FxAppController extends FileMenuController implements IDocumentList
private void handleMarkerDragged(final Node node, final double dx, final double dy) {
final MapProjection projection = mapView.getProjection();
- final Point2D point = projection.locationToViewportPoint(marker.getLocation());
+ final Point2D point = projection.locationToViewportPoint(marker.get().getLocation());
final Location newLocation = projection.viewportPointToLocation(point.add(dx, dy));
getLatLongs().setLatLong(locationListView.getSelectionModel().getSelectedIndex(), location2LatLong(newLocation));
updateLocationViewListSelection(false);
@@ -96,22 +97,23 @@ public class FxAppController extends FileMenuController implements IDocumentList
if (num < 0 || num >= getLatLongs().getLatLongCount()) {
markersParent.getItems().clear();
if (draggableMarkerController != null) {
- draggableMarkerController.detach(marker);
+ draggableMarkerController.detach(marker.get());
}
- marker = null;
+ marker = Optional.empty();
} else {
final LatLong latLong = getLatLongs().getLatLong(num);
- if (marker == null) {
- marker = new MapMarker(latLong);
- markersParent.getItems().add(marker);
+ if (marker.isEmpty()) {
+ final MapMarker aMarker = new MapMarker(latLong);
+ markersParent.getItems().add(aMarker);
if (draggableMarkerController != null) {
- draggableMarkerController.attach(marker);
+ draggableMarkerController.attach(aMarker);
}
+ marker = Optional.of(aMarker);
} else {
- marker.setLocation(latLong);
+ marker.get().setLocation(latLong);
}
if (centerOnMarker) {
- mapView.setCenter(marker.getLocation());
+ mapView.setCenter(marker.get().getLocation());
}
}
}
diff --git a/simpleexample/src/main/java/simpleex/ui/MapMarker.java b/simpleexample/src/main/java/simpleex/ui/MapMarker.java
index f61ce55..a2365b7 100644
--- a/simpleexample/src/main/java/simpleex/ui/MapMarker.java
+++ b/simpleexample/src/main/java/simpleex/ui/MapMarker.java
@@ -16,7 +16,7 @@ public class MapMarker extends MapItem<LatLong> {
getChildren().add(circle);
}
- public void setLocation(final LatLong latLong) {
- setLocation(new Location(latLong.latitude, latLong.longitude));
+ public final void setLocation(final LatLong latLong) {
+ setLocation(new Location(latLong.getLatitude(), latLong.getLongitude()));
}
}
diff --git a/simpleexample/src/test/java/simpleex/core/LatLongTest.java b/simpleexample/src/test/java/simpleex/core/LatLongTest.java
index b6a3335..c51d3ac 100644
--- a/simpleexample/src/test/java/simpleex/core/LatLongTest.java
+++ b/simpleexample/src/test/java/simpleex/core/LatLongTest.java
@@ -19,8 +19,8 @@ public class LatLongTest {
}
private void testLatLong(final LatLong latLong, final double lat, final double lon) {
- Assert.assertEquals(lat, latLong.latitude, 0.0);
- Assert.assertEquals(lon, latLong.longitude, 0.0);
+ Assert.assertEquals(lat, latLong.getLatitude(), 0.0);
+ Assert.assertEquals(lon, latLong.getLongitude(), 0.0);
}
@Test
diff --git a/simpleexample/src/test/java/simpleex/core/LatLongsTest.java b/simpleexample/src/test/java/simpleex/core/LatLongsTest.java
index 0c7d4f2..55566eb 100644
--- a/simpleexample/src/test/java/simpleex/core/LatLongsTest.java
+++ b/simpleexample/src/test/java/simpleex/core/LatLongsTest.java
@@ -4,14 +4,12 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
-import simpleex.core.LatLongs;
-
public class LatLongsTest {
private LatLongs latLongs;
@Before
- public void setup() {
+ public void setUp() {
latLongs = new LatLongs();
}
diff --git a/simpleexample/src/test/java/simpleex/json/AbstractJsonTest.java b/simpleexample/src/test/java/simpleex/json/AbstractJsonTest.java
index cfbd5e6..6b84096 100644
--- a/simpleexample/src/test/java/simpleex/json/AbstractJsonTest.java
+++ b/simpleexample/src/test/java/simpleex/json/AbstractJsonTest.java
@@ -19,7 +19,7 @@ public abstract class AbstractJsonTest {
return objectMapper;
}
- public void setup() {
+ protected void setUp() {
objectMapper = createObjectMapper();
}
diff --git a/simpleexample/src/test/java/simpleex/json/LatLongJsonTest.java b/simpleexample/src/test/java/simpleex/json/LatLongJsonTest.java
index 7e926a5..8add899 100644
--- a/simpleexample/src/test/java/simpleex/json/LatLongJsonTest.java
+++ b/simpleexample/src/test/java/simpleex/json/LatLongJsonTest.java
@@ -7,15 +7,13 @@ import org.junit.Test;
import com.fasterxml.jackson.databind.ObjectMapper;
import simpleex.core.LatLong;
-import simpleex.json.LatLongDeserializer;
-import simpleex.json.LatLongSerializer;
public class LatLongJsonTest extends AbstractJsonTest {
@Before
@Override
- public void setup() {
- super.setup();
+ public void setUp() {
+ super.setUp();
}
@Override
diff --git a/simpleexample/src/test/java/simpleex/ui/FxAppTest.java b/simpleexample/src/test/java/simpleex/ui/FxAppTest.java
index fad97d3..bdeea45 100644
--- a/simpleexample/src/test/java/simpleex/ui/FxAppTest.java
+++ b/simpleexample/src/test/java/simpleex/ui/FxAppTest.java
@@ -72,7 +72,7 @@ public class FxAppTest extends ApplicationTest {
@Test
public void testController() {
- Assert.assertTrue(this.controller instanceof FxAppController);
+ Assert.assertNotNull(this.controller);
}
@Test
@@ -90,8 +90,8 @@ public class FxAppTest extends ApplicationTest {
// center of map view is approx. the first LatLong object
final Location center = mapView.getCenter();
final double epsilon = 0.000001; // round-off error
- Assert.assertEquals(latLongList.get(0).latitude, center.getLatitude(), epsilon);
- Assert.assertEquals(latLongList.get(0).longitude, center.getLongitude(), epsilon);
+ Assert.assertEquals(latLongList.get(0).getLatitude(), center.getLatitude(), epsilon);
+ Assert.assertEquals(latLongList.get(0).getLongitude(), center.getLongitude(), epsilon);
}
@Test
--
GitLab