Changeset 31812 in osm for applications/editors/josm/plugins

Timestamp:

2015-12-11T22:41:08+01:00 (9 years ago)

Author:

floscher

Message:

[mapillary] Fix some more issues reported by static code analysis

Location:

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary

Files:

• MapillaryAbstractImage.java (modified) (9 diffs)
• MapillaryPlugin.java (modified) (3 diffs)
• actions/MapillaryUploadAction.java (modified) (1 diff)
• gui/MapillaryFilterDialog.java (modified) (13 diffs)

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/MapillaryAbstractImage.java

@@ -17 +17 @@
  *
  */
-public abstract class MapillaryAbstractImage {
+public class MapillaryAbstractImage {
   /**
    * If two values for field ca differ by less than EPSILON both values are considered equal.
@@ -58 +58 @@
    *          The direction of the picture (0 means north).
    */
-  public MapillaryAbstractImage(double lat, double lon, double ca) {
+  protected MapillaryAbstractImage(final double lat, final double lon, final double ca) {
     this.latLon = new LatLon(lat, lon);
     this.tempLatLon = this.latLon;
@@ -92 +92 @@
    */
   public String getDate() {
-    StringBuilder format = new StringBuilder(26);
-    if (Main.pref.getBoolean("iso.dates"))
+    final StringBuilder format = new StringBuilder(26);
+    if (Main.pref.getBoolean("iso.dates")) {
       format.append("yyyy-MM-dd");
-    else
+    } else {
       format.append("dd/MM/yyyy");
+    }
     if (Main.pref.getBoolean("mapillary.display-hour", true)) {
-      if (Main.pref.getBoolean("mapillary.format-24"))
+      if (Main.pref.getBoolean("mapillary.format-24")) {
         format.append(" - HH:mm:ss (z)");
-      else
+      } else {
         format.append(" - h:mm:ss a (z)");
+      }
     }
     return getDate(format.toString());
@@ -116 +118 @@
    */
   public String getDate(String format) {
-    Date date = new Date(getCapturedAt());
-
-    SimpleDateFormat formatter = new SimpleDateFormat(format, Locale.UK);
+    final Date date = new Date(getCapturedAt());
+
+    final SimpleDateFormat formatter = new SimpleDateFormat(format, Locale.UK);
     formatter.setTimeZone(Calendar.getInstance().getTimeZone());
     return formatter.format(date);
@@ -186 +188 @@
    * Moves the image temporally to another position
    *
-   * @param x
-   *          The movement of the image in longitude units.
-   * @param y
-   *          The movement of the image in latitude units.
-   */
-  public void move(double x, double y) {
-    this.movingLatLon = new LatLon(this.tempLatLon.getY() + y,
-        this.tempLatLon.getX() + x);
+   * @param x The movement of the image in longitude units.
+   * @param y The movement of the image in latitude units.
+   */
+  public void move(final double x, final double y) {
+    this.movingLatLon = new LatLon(this.tempLatLon.getY() + y, this.tempLatLon.getX() + x);
   }
 
@@ -227 +226 @@
    * Sets the Epoch time when the picture was captured.
    *
-   * @param capturedAt
-   *          Epoch time when the image was captured.
-   */
-  public void setCapturedAt(long capturedAt) {
+   * @param capturedAt Epoch time when the image was captured.
+   */
+  public void setCapturedAt(final long capturedAt) {
     this.capturedAt = capturedAt;
   }
@@ -237 +235 @@
    * Sets the MapillarySequence object which contains the MapillaryImage.
    *
-   * @param sequence
-   *          The MapillarySequence that contains the MapillaryImage.
-   */
-  public void setSequence(MapillarySequence sequence) {
+   * @param sequence The MapillarySequence that contains the MapillaryImage.
+   */
+  public void setSequence(final MapillarySequence sequence) {
     this.sequence = sequence;
   }
@@ -247 +244 @@
    * Set's whether the image should be visible on the map or not.
    *
-   * @param visible
-   *          true if the image is set to be visible; false otherwise.
-   */
-  public void setVisible(boolean visible) {
+   * @param visible true if the image is set to be visible; false otherwise.
+   */
+  public void setVisible(final boolean visible) {
     this.visible = visible;
   }
@@ -266 +262 @@
    * Turns the image direction.
    *
-   * @param ca
-   *          The angle the image is moving.
-   */
-  public void turn(double ca) {
+   * @param ca The angle the image is moving.
+   */
+  public void turn(final double ca) {
     this.movingCa = this.tempCa + ca;
   }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/MapillaryPlugin.java

@@ -61 +61 @@
   public static CacheAccess<String, BufferedImageCacheEntry> CACHE;
 
-  private final MapillaryDownloadAction downloadAction;
-  private final MapillaryExportAction exportAction;
+  private static final MapillaryDownloadAction downloadAction = new MapillaryDownloadAction();
+  private static final MapillaryExportAction exportAction = new MapillaryExportAction();
   /** Import action */
-  private final MapillaryImportAction importAction;
+  private static final MapillaryImportAction importAction = new MapillaryImportAction();
   /** Zoom action */
-  private static MapillaryZoomAction zoomAction;
-  private final MapillaryDownloadViewAction downloadViewAction;
-  private final MapillaryImportIntoSequenceAction importIntoSequenceAction;
-  private final MapillaryJoinAction joinAction;
+  private static final MapillaryZoomAction zoomAction = new MapillaryZoomAction();
+  private static final MapillaryDownloadViewAction downloadViewAction = new MapillaryDownloadViewAction();
+  private static final MapillaryImportIntoSequenceAction importIntoSequenceAction = new MapillaryImportIntoSequenceAction();
+  private static final MapillaryJoinAction joinAction = new MapillaryJoinAction();
   /** Walk action */
-  private static MapillaryWalkAction walkAction;
+  private static final MapillaryWalkAction walkAction = new MapillaryWalkAction();
   /** Upload action */
-  private static MapillaryUploadAction uploadAction;
+  private static final MapillaryUploadAction uploadAction = new MapillaryUploadAction();
 
   /** Menu button for the {@link MapillaryDownloadAction} action. */
@@ -103 +103 @@
     super(info);
 
-    downloadAction = new MapillaryDownloadAction();
-    walkAction = new MapillaryWalkAction();
-    exportAction = new MapillaryExportAction();
-    importAction = new MapillaryImportAction();
-    zoomAction = new MapillaryZoomAction();
-    downloadViewAction = new MapillaryDownloadViewAction();
-    importIntoSequenceAction = new MapillaryImportIntoSequenceAction();
-    joinAction = new MapillaryJoinAction();
-    uploadAction = new MapillaryUploadAction();
-
     if (Main.main != null) { // important for headless mode
-      downloadMenu = MainMenu.add(Main.main.menu.imageryMenu, this.downloadAction, false);
+      downloadMenu = MainMenu.add(Main.main.menu.imageryMenu, downloadAction, false);
       exportMenu = MainMenu.add(Main.main.menu.fileMenu, exportAction, false, 14);
       importIntoSequenceMenu = MainMenu.add(Main.main.menu.fileMenu, importIntoSequenceAction, false, 14);
@@ -120 +110 @@
       uploadMenu = MainMenu.add(Main.main.menu.fileMenu, uploadAction, false, 14);
       zoomMenu = MainMenu.add(Main.main.menu.viewMenu, zoomAction, false, 15);
-      downloadViewMenu = MainMenu.add(Main.main.menu.fileMenu, this.downloadViewAction, false, 14);
-      joinMenu = MainMenu.add(Main.main.menu.dataMenu, this.joinAction, false);
+      downloadViewMenu = MainMenu.add(Main.main.menu.fileMenu, downloadViewAction, false, 14);
+      joinMenu = MainMenu.add(Main.main.menu.dataMenu, joinAction, false);
       walkMenu = MainMenu.add(Main.main.menu.moreToolsMenu, walkAction, false);
 

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/MapillaryUploadAction.java

@@ -55 +55 @@
     if (pane.getValue() != null
         && (int) pane.getValue() == JOptionPane.OK_OPTION
-        && dialog.delete != null) {
-      if (dialog.sequence.isSelected()) {
-        UploadUtils.uploadSequence(MapillaryLayer.getInstance().getData()
-            .getSelectedImage().getSequence(), dialog.delete.isSelected());
-      }
+        && dialog.delete != null
+        && dialog.sequence.isSelected()) {
+      UploadUtils.uploadSequence(
+          MapillaryLayer.getInstance().getData().getSelectedImage().getSequence(),
+          dialog.delete.isSelected()
+      );
     }
   }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/gui/MapillaryFilterDialog.java

@@ -40 +40 @@
  *
  */
-public class MapillaryFilterDialog extends ToggleDialog implements
-    MapillaryDataListener {
+public class MapillaryFilterDialog extends ToggleDialog implements MapillaryDataListener {
 
   private static final long serialVersionUID = -4192029663670922103L;
 
-  private static MapillaryFilterDialog INSTANCE;
-
-  private static final String[] TIME_LIST = { tr("All"), tr("Years"),
-      tr("Months"), tr("Days") };
+  private static MapillaryFilterDialog instance;
+
+  private static final String[] TIME_LIST = { tr("All"), tr("Years"), tr("Months"), tr("Days") };
 
   private final JPanel panel;
 
   /** Spinner to choose the range of dates. */
-  public SpinnerNumberModel spinner;
+  private final SpinnerNumberModel spinner;
 
   private final JCheckBox imported = new JCheckBox(tr("Imported images"));
-  private final JCheckBox downloaded = new JCheckBox(
-      new downloadCheckBoxAction());
+  private final JCheckBox downloaded = new JCheckBox(new DownloadCheckBoxAction());
   private final JCheckBox onlySigns = new JCheckBox(new OnlySignsAction());
   private final JComboBox<String> time;
@@ -66 +63 @@
   private final JButton signChooser = new JButton(new SignChooserAction());
 
-  private final MapillaryFilterChooseSigns signFilter = MapillaryFilterChooseSigns
-      .getInstance();
+  private final MapillaryFilterChooseSigns signFilter = MapillaryFilterChooseSigns.getInstance();
 
   /** The list of sign names */
-  private final String[] SIGN_TAGS = { "prohibitory_speed_limit",
+  private static final String[] SIGN_TAGS = { "prohibitory_speed_limit",
       "priority_stop", "other_give_way", "mandatory_roundabout",
       "other_no_entry", "prohibitory_no_traffic_both_ways",
@@ -78 +74 @@
       "danger_pedestrian_crossing", "prohibitory_no_u_turn",
       "prohibitory_noturn" };
-  /** The the {@link JCheckBox} where the respective tag should be searched */
+  /** The {@link JCheckBox} where the respective tag should be searched */
   private final JCheckBox[] SIGN_CHECKBOXES = { this.signFilter.maxSpeed,
       this.signFilter.stop, this.signFilter.giveWay,
@@ -137 +133 @@
 
   /**
-   * Returns the unique instance of the class.
-   *
-   * @return THe unique instance of the class.
-   */
-  public static MapillaryFilterDialog getInstance() {
-    if (INSTANCE == null)
-      INSTANCE = new MapillaryFilterDialog();
-    return INSTANCE;
+   * @return the unique instance of the class.
+   */
+  public static synchronized MapillaryFilterDialog getInstance() {
+    if (instance == null)
+      instance = new MapillaryFilterDialog();
+    return instance;
   }
 
@@ -153 +147 @@
 
   @Override
-  public void selectedImageChanged(MapillaryAbstractImage oldImage,
-      MapillaryAbstractImage newImage) {
+  public void selectedImageChanged(MapillaryAbstractImage oldImage, MapillaryAbstractImage newImage) {
+    // Do nothing when image selection changed
   }
 
@@ -179 +173 @@
     boolean onlySigns = this.onlySigns.isSelected();
 
-    for (MapillaryAbstractImage img : MapillaryLayer.getInstance().getData()
-        .getImages()) {
+    for (MapillaryAbstractImage img : MapillaryLayer.getInstance().getData().getImages()) {
       img.setVisible(true);
       if (img instanceof MapillaryImportedImage) {
@@ -201 +194 @@
           }
         }
-        if (!this.user.getText().equals("")
+        if (!"".equals(user.getText())
             && !this.user.getText().equals(((MapillaryImage) img).getUser())) {
           img.setVisible(false);
@@ -209 +202 @@
       // Calculates the amount of days since the image was taken
       Long currentTime = currentTime();
-      if (this.time.getSelectedItem().equals(TIME_LIST[1])) {
-        if (img.getCapturedAt() < currentTime
-            - ((Integer) this.spinner.getValue()).longValue() * 365 * 24 * 60
-            * 60 * 1000) {
+      long[] timeFactor = new long[]{
+          31536000000L, // = 365 * 24 * 60 * 60 * 1000 = number of ms in a year
+          2592000000L, // = 30 * 24 * 60 * 60 * 1000 = number of ms in a month
+          86400000 // = 24 * 60 * 60 * 1000 = number of ms in a day
+      };
+      for (int i = 1; i <= 3; i++) {
+        if (TIME_LIST[i].equals(time.getSelectedItem())
+            && img.getCapturedAt() < currentTime - ((Integer) spinner.getValue()).longValue() * timeFactor[i - 1]
+        ) {
           img.setVisible(false);
-          continue;
-        }
-      }
-      if (this.time.getSelectedItem().equals(TIME_LIST[2])) {
-        if (img.getCapturedAt() < currentTime
-            - ((Integer) this.spinner.getValue()).longValue() * 30 * 24 * 60
-            * 60 * 1000) {
-          img.setVisible(false);
-          continue;
-        }
-      }
-      if (this.time.getSelectedItem().equals(TIME_LIST[3])) {
-        if (img.getCapturedAt() < currentTime
-            - ((Integer) this.spinner.getValue()).longValue() * 60 * 60 * 1000) {
-          img.setVisible(false);
-          continue;
         }
       }
@@ -246 +228 @@
    */
   private boolean checkSigns(MapillaryImage img) {
-    for (int i = 0; i < this.SIGN_TAGS.length; i++) {
-      if (checkSign(img, this.SIGN_CHECKBOXES[i], this.SIGN_TAGS[i]))
+    for (int i = 0; i < SIGN_TAGS.length; i++) {
+      if (checkSign(img, this.SIGN_CHECKBOXES[i], SIGN_TAGS[i]))
         return true;
     }
@@ -270 +252 @@
   }
 
-  private class downloadCheckBoxAction extends AbstractAction {
+  private class DownloadCheckBoxAction extends AbstractAction {
 
     private static final long serialVersionUID = 4672634002899519496L;
 
-    public downloadCheckBoxAction() {
+    public DownloadCheckBoxAction() {
       putValue(NAME, tr("Downloaded images"));
     }
@@ -280 +262 @@
     @Override
     public void actionPerformed(ActionEvent arg0) {
-      MapillaryFilterDialog.this.onlySigns
-          .setEnabled(MapillaryFilterDialog.this.downloaded.isSelected());
+      onlySigns.setEnabled(downloaded.isSelected());
     }
   }
@@ -325 +306 @@
     @Override
     public void actionPerformed(ActionEvent arg0) {
-      MapillaryFilterDialog.this.signChooser
-          .setEnabled(MapillaryFilterDialog.this.onlySigns.isSelected());
+      signChooser.setEnabled(onlySigns.isSelected());
     }
   }
@@ -360 +340 @@
    * Destroys the unique instance of the class.
    */
-  public static void destroyInstance() {
-    MapillaryFilterDialog.INSTANCE = null;
+  public static synchronized void destroyInstance() {
+    instance = null;
   }
 }