Changeset 36382 in osm for applications

Timestamp:

2025-02-04T20:37:26+01:00 (13 hours ago)

Author:

taylor.smock

Message:

Fix most sonarlint issues in buildings_tools

The remaining issues are as follows:

• Package name (buildings_tools)
• Some documentation for public methods where the purpose of the method was unclear
• A loop which (according to sonarlint) should be combined with another loop
  • This isn't the case -- there are side effects from the loop.

Location:

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

Files:

• AddressDialog.java (modified) (4 diffs)
• AdvancedSettingsDialog.java (modified) (2 diffs)
• AngleSnap.java (modified) (5 diffs)
• Building.java (modified) (12 diffs)
• BuildingCircleAction.java (modified) (1 diff)
• BuildingRectangleAction.java (modified) (1 diff)
• BuildingSizeAction.java (modified) (2 diffs)
• BuildingSizeDialog.java (modified) (8 diffs)
• DrawBuildingAction.java (modified) (24 diffs)
• MergeAddrPointsAction.java (modified) (9 diffs)
• MyDialog.java (modified) (3 diffs)

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/AddressDialog.java

@@ -11 +11 @@
 import javax.swing.SpinnerNumberModel;
 
+/**
+ * Address dialog for buildings
+ */
 public class AddressDialog extends MyDialog {
-    private static String lhousenum, lstreetname;
-    private static int inc = 0;
-    private JTextField housenum = new JTextField();
-    private JTextField streetname = new JTextField();
-    private JSpinner incSpinner;
+    private static String lhousenum;
+    private static String lstreetname;
+    private static int inc;
+    private final JTextField housenum = new JTextField();
+    private final JTextField streetname = new JTextField();
+    private final JSpinner incSpinner;
 
+    /**
+     * Create a new dialog object
+     */
     public AddressDialog() {
         super(tr("Building address"));
@@ -26 +33 @@
         streetname.setText(lstreetname);
 
-        SpinnerNumberModel inc_model = new SpinnerNumberModel(0, Integer.MIN_VALUE, Integer.MAX_VALUE, 1);
-        incSpinner = new JSpinner(inc_model);
+        SpinnerNumberModel incModel = new SpinnerNumberModel(0, Integer.MIN_VALUE, Integer.MAX_VALUE, 1);
+        incSpinner = new JSpinner(incModel);
         incSpinner.setValue(inc);
         addLabelled(tr("House number increment:"), incSpinner);
@@ -39 +46 @@
             return "";
         try {
-            Integer num = NumberFormat.getInstance().parse(lhousenum).intValue() + inc;
-            return num.toString();
+            int num = NumberFormat.getInstance().parse(lhousenum).intValue() + inc;
+            return Integer.toString(num);
         } catch (ParseException e) {
             return lhousenum;
@@ -46 +53 @@
     }
 
+    /**
+     * Save values for future use in the current running JOSM instance
+     */
     public final void saveValues() {
-        lhousenum = housenum.getText();
-        lstreetname = streetname.getText();
-        inc = (Integer) incSpinner.getValue();
+        saveValues(this);
+    }
+
+    private static void saveValues(AddressDialog dialog) {
+        lhousenum = dialog.getHouseNum();
+        lstreetname = dialog.getStreetName();
+        inc = (Integer) dialog.incSpinner.getValue();
     }
 

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/AdvancedSettingsDialog.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.awt.GridBagConstraints;
 import java.util.Map;
 
@@ -31 +32 @@
         super(tr("Advanced settings"));
 
-        panel.add(new JLabel(tr("Buildings tags:")), GBC.eol().fill(GBC.HORIZONTAL));
+        panel.add(new JLabel(tr("Buildings tags:")), GBC.eol().fill(GridBagConstraints.HORIZONTAL));
 
         for (Map.Entry<String, String> entry : ToolSettings.getTags().entrySet()) {
             tagsModel.add(entry.getKey(), entry.getValue());
         }
-        panel.add(new TagEditorPanel(tagsModel, null, Changeset.MAX_CHANGESET_TAG_LENGTH), GBC.eop().fill(GBC.BOTH));
+        panel.add(new TagEditorPanel(tagsModel, null, Changeset.MAX_CHANGESET_TAG_LENGTH), GBC.eop().fill(GridBagConstraints.BOTH));
 
-        panel.add(cBigMode, GBC.eol().fill(GBC.HORIZONTAL));
-        panel.add(cSoftCur, GBC.eol().fill(GBC.HORIZONTAL));
-        panel.add(cNoClickDrag, GBC.eol().fill(GBC.HORIZONTAL));
-        panel.add(cToggleMapMode, GBC.eol().fill(GBC.HORIZONTAL));
+        panel.add(cBigMode, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
+        panel.add(cSoftCur, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
+        panel.add(cNoClickDrag, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
+        panel.add(cToggleMapMode, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
 
         cBigMode.setSelected(ToolSettings.isBBMode());

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/AngleSnap.java

@@ -11 +11 @@
 import org.openstreetmap.josm.tools.Pair;
 
+/**
+ * A class for getting angles for snapping
+ */
 public class AngleSnap {
     private static final double PI_2 = Math.PI / 2;
     final TreeSet<Double> snapSet = new TreeSet<>();
 
+    /**
+     * Clear the snap directions
+     */
     public final void clear() {
         snapSet.clear();
     }
 
+    /**
+     * Add a snap direction
+     * @param snap The direction in radians
+     */
     public final void addSnap(double snap) {
         snapSet.add(snap % PI_2);
     }
 
+    /**
+     * Add node pairs to the possible snap directions (the heading and 45 degrees from heading)
+     * @param nodes The nodes to iterate through
+     * @return The heading between the nodes in radians
+     */
     public final Double addSnap(Node[] nodes) {
         if (nodes.length == 2) {
@@ -36 +51 @@
     }
 
+    /**
+     * Add way segments to the possible snap directions
+     * @param way The way to add node pairs from to the snap directions
+     */
     public final void addSnap(Way way) {
         for (Pair<Node, Node> pair : way.getNodePairs(false)) {
@@ -45 +64 @@
     }
 
+    /**
+     * Get the angle between the first and last snaps added
+     * @return The angle between the first and last snaps added
+     */
     public final Double getAngle() {
         if (snapSet.isEmpty()) {
@@ -64 +87 @@
     }
 
+    /**
+     * Get the snap angle given a starting angle
+     * @param angle The angle to get the snap for
+     * @return The best angle for snapping, or the originating angle
+     */
     public final double snapAngle(double angle) {
         if (snapSet.isEmpty()) {
@@ -78 +106 @@
 
         if (Math.abs(ang - next) > Math.abs(ang - prev)) {
-            if (Math.abs(ang - prev) > Math.PI / 8) {
-                return angle;
-            } else {
-                double ret = prev + PI_2 * quadrant;
-                if (ret < 0)
-                    ret += 2 * Math.PI;
-                return ret;
-            }
+            return prevSnapAngle(quadrant, prev, ang, angle);
         } else {
-            if (Math.abs(ang - next) > Math.PI / 8) {
-                return angle;
-            } else {
-                double ret = next + PI_2 * quadrant;
-                if (ret > 2 * Math.PI)
-                    ret -= 2 * Math.PI;
-                return ret;
-            }
+            return nextSnapAngle(quadrant, next, ang, angle);
+        }
+    }
+
+    private static double nextSnapAngle(double quadrant, double next, double ang, double angle) {
+        if (Math.abs(ang - next) > Math.PI / 8) {
+            return angle;
+        } else {
+            double ret = next + PI_2 * quadrant;
+            if (ret > 2 * Math.PI)
+                ret -= 2 * Math.PI;
+            return ret;
+        }
+    }
+
+    private static double prevSnapAngle(double quadrant, double prev, double ang, double angle) {
+        if (Math.abs(ang - prev) > Math.PI / 8) {
+            return angle;
+        } else {
+            double ret = prev + PI_2 * quadrant;
+            if (ret < 0)
+                ret += 2 * Math.PI;
+            return ret;
         }
     }

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/Building.java

@@ -31 +31 @@
 import org.openstreetmap.josm.data.osm.BBox;
 import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.IPrimitive;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -53 +54 @@
     private static final double EQUAL_NODE_DIST_TOLERANCE = 1e-6;
 
+    /**
+     * Clear the current angle information
+     */
     public void clearAngleSnap() {
         angleSnap.clear();
@@ -58 +62 @@
     }
 
+    /**
+     * Add nodes to the angle snap collection and set the current drawing angle to the heading between
+     * the nodes.
+     * @param nodes The node array to use
+     * @see AngleSnap#addSnap(Node[])
+     */
     public void addAngleSnap(Node[] nodes) {
         drawingAngle = angleSnap.addSnap(nodes);
     }
 
+    /**
+     * Add a way to the angle snap collection and set the drawing angle if it is not already set
+     * @param way The way to add to the angle snap
+     * @see AngleSnap#addSnap(Way)
+     */
     public void addAngleSnap(Way way) {
         angleSnap.addSnap(way);
@@ -77 +92 @@
     }
 
+    /**
+     * Check if we are currently drawing a rectangle
+     * @return {@code true} if we are drawing a rectangle
+     */
     public boolean isRectDrawing() {
         return drawingAngle != null && ToolSettings.getWidth() == 0 && ToolSettings.getLenStep() == 0
@@ -82 +101 @@
     }
 
+    /**
+     * Get the current drawing angle
+     * @return The current drawing angle. May be {@code null}.
+     */
     public Double getDrawingAngle() {
         return drawingAngle;
     }
 
+    /**
+     * Reset the current building drawing
+     */
     public void reset() {
         len = 0;
@@ -94 +120 @@
     }
 
+    /**
+     * Get the east north point specified
+     * @param num The point to get from the {@link #en} array
+     * @return The EastNorth point
+     */
     public EastNorth getPoint(int num) {
         return en[num];
@@ -209 +240 @@
     }
 
+    /**
+     * Paint the way to be generated on mouse release
+     * @param g The graphics to paint on
+     * @param mv The current map view
+     */
     public void paint(Graphics2D g, MapView mv) {
         // Make a local copy to avoid other threads resetting what we are drawing.
@@ -255 +291 @@
             if (n.isUsable() && findUsableTag(n)) {
                 EastNorth enTest = n.getEastNorth();
-                if (area.contains(enTest.getX(), enTest.getY())) {
-                    boolean useNode = true;
-                    for (OsmPrimitive p : n.getReferrers()) {
-                        // Don't use nodes if they're referenced by ways
-                        if (p.getType() == OsmPrimitiveType.WAY) {
-                            useNode = false;
-                            break;
-                        }
-                    }
-                    if (useNode) {
-                        nodes.add(n);
-                    }
+                if (area.contains(enTest.getX(), enTest.getY()) && usableAddressNode(n)) {
+                    nodes.add(n);
                 }
             }
@@ -273 +299 @@
             return null;
         return nodes.get(0);
+    }
+
+    private static boolean usableAddressNode(IPrimitive node) {
+        for (IPrimitive p : node.getReferrers()) {
+            // Don't use nodes if they're referenced by ways
+            if (p.getType() == OsmPrimitiveType.WAY) {
+                return false;
+            }
+        }
+        return true;
     }
 
@@ -284 +320 @@
     }
 
+    /**
+     * Create a circle in the dataset
+     * @return The created circle
+     */
     public Way createCircle() {
         DataSet ds = MainApplication.getLayerManager().getEditDataSet();
@@ -358 +398 @@
     }
 
+    /**
+     * Create a rectangle on the current edit dataset
+     * @param ctrl {@code true} if the ctrl button is held
+     * @return The created rectangle or {@code null}
+     */
     public Way createRectangle(boolean ctrl) {
         if (len == 0)
@@ -474 +519 @@
             Node addrNode = getAddressNode(w);
             if (addrNode != null) {
-                Collection<Command> addressCmds = cmdList != null ? cmdList : new LinkedList<>();
-                addrNode.getKeys().forEach(w::put);
-                for (OsmPrimitive p : addrNode.getReferrers()) {
-                    Relation r = (Relation) p;
-                    List<RelationMember> members = new ArrayList<>(r.getMembers());
-                    for (int i = 0; i < members.size(); i++) {
-                        RelationMember member = members.get(i);
-                        if (addrNode.equals(member.getMember())) {
-                            members.set(i, new RelationMember(member.getRole(), w));
-                        }
-                    }
-                    addressCmds.add(new ChangeMembersCommand(r, members));
+                addAddressCommand(w, cmdList, addrNode);
+            }
+        }
+    }
+
+    private static void addAddressCommand(Way w, Collection<Command> cmdList, Node addrNode) {
+        Collection<Command> addressCmds = cmdList != null ? cmdList : new LinkedList<>();
+        addrNode.getKeys().forEach(w::put);
+        for (OsmPrimitive p : addrNode.getReferrers()) {
+            Relation r = (Relation) p;
+            List<RelationMember> members = new ArrayList<>(r.getMembers());
+            for (int i = 0; i < members.size(); i++) {
+                RelationMember member = members.get(i);
+                if (addrNode.equals(member.getMember())) {
+                    members.set(i, new RelationMember(member.getRole(), w));
                 }
-                final Command deleteCommand = DeleteCommand.delete(Collections.singleton(addrNode));
-                if (deleteCommand != null) {
-                    addressCmds.add(deleteCommand);
-                }
-                if (cmdList == null) {
-                    Command c = new SequenceCommand(tr("Add address for building"), addressCmds);
-                    UndoRedoHandler.getInstance().add(c);
-                }
-            }
+            }
+            addressCmds.add(new ChangeMembersCommand(r, members));
+        }
+        final Command deleteCommand = DeleteCommand.delete(Collections.singleton(addrNode));
+        if (deleteCommand != null) {
+            addressCmds.add(deleteCommand);
+        }
+        if (cmdList == null) {
+            Command c = new SequenceCommand(tr("Add address for building"), addressCmds);
+            UndoRedoHandler.getInstance().add(c);
         }
     }

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/BuildingCircleAction.java

@@ -10 +10 @@
 import org.openstreetmap.josm.tools.Shortcut;
 
+/**
+ * The action for drawing building circles
+ */
 public class BuildingCircleAction extends JosmAction {
 
+    /**
+     * Create a new building circle action
+     */
     public BuildingCircleAction() {
         super(tr("Set building shape to circle"), "mapmode/silo", tr("Set building shape to circle"),

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/BuildingRectangleAction.java

@@ -10 +10 @@
 import org.openstreetmap.josm.tools.Shortcut;
 
+/**
+ * Draw rectangle buildings
+ */
 public class BuildingRectangleAction extends JosmAction {
 
+    /**
+     * Create a new action for drawing rectangular buildings
+     */
     public BuildingRectangleAction() {
         super(tr("Set building shape to rectangle"), "mapmode/rectangular", tr("Set building shape to rectangle"),

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/BuildingSizeAction.java

@@ -2 +2 @@
 package org.openstreetmap.josm.plugins.buildings_tools;
 
+import static org.openstreetmap.josm.tools.I18n.marktr;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
@@ -10 +11 @@
 import org.openstreetmap.josm.tools.Shortcut;
 
+/**
+ * An action to open the {@link BuildingSizeDialog}
+ */
 public class BuildingSizeAction extends JosmAction {
-
+    private static final String SET_BUILDINGS_SIZE = marktr("Set buildings size");
+    /**
+     * Create a new action for setting building sizes
+     */
     public BuildingSizeAction() {
-        super(tr("Set buildings size"), "mapmode/building", tr("Set buildings size"),
-                Shortcut.registerShortcut("edit:buildingsdialog", tr("Data: {0}", tr("Set buildings size")),
+        super(tr(SET_BUILDINGS_SIZE), "mapmode/building", tr(SET_BUILDINGS_SIZE),
+                Shortcut.registerShortcut("edit:buildingsdialog", tr("Data: {0}", tr(SET_BUILDINGS_SIZE)),
                 KeyEvent.VK_B, Shortcut.ALT_CTRL),
                 true, "edit:buildingsdialog", false);

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/BuildingSizeDialog.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
-import java.awt.event.ActionEvent;
-import java.awt.event.ActionListener;
+import java.awt.GridBagConstraints;
 import java.text.NumberFormat;
 import java.text.ParseException;
@@ -17 +16 @@
 import org.openstreetmap.josm.tools.GBC;
 
+/**
+ * A dialog for setting building sizes and other information
+ */
 public class BuildingSizeDialog extends MyDialog {
     private final JFormattedTextField twidth = new JFormattedTextField(NumberFormat.getInstance());
@@ -27 +29 @@
     private final JRadioButton rectangleRadio = new JRadioButton(tr("Rectangle"));
 
+    /**
+     * Create a new dialog for building sizes
+     */
     public BuildingSizeDialog() {
         super(tr("Set buildings size and shape"));
@@ -36 +41 @@
         rectangleRadio.setSelected(ToolSettings.Shape.RECTANGLE == ToolSettings.getShape());
 
-        panel.add(rectangleRadio, GBC.eol().fill(GBC.HORIZONTAL));
-        panel.add(circleRadio, GBC.eol().fill(GBC.HORIZONTAL));
+        panel.add(rectangleRadio, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
+        panel.add(circleRadio, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
 
         addLabelled(tr("Buildings width/diameter:"), twidth);
         addLabelled(tr("Length step:"), tlenstep);
-        panel.add(caddr, GBC.eol().fill(GBC.HORIZONTAL));
-        panel.add(cAutoSelect, GBC.eol().fill(GBC.HORIZONTAL));
-        panel.add(cAutoSelectReplaceSelection, GBC.eol().fill(GBC.HORIZONTAL));
-        panel.add(cAddrNode, GBC.eol().fill(GBC.HORIZONTAL));
+        panel.add(caddr, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
+        panel.add(cAutoSelect, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
+        panel.add(cAutoSelectReplaceSelection, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
+        panel.add(cAddrNode, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
 
         twidth.setValue(ToolSettings.getWidth());
@@ -54 +59 @@
 
         JButton bAdv = new JButton(tr("Advanced..."));
-        bAdv.addActionListener(new ActionListener() {
-            @Override
-            public void actionPerformed(ActionEvent arg0) {
-                AdvancedSettingsDialog dlg = new AdvancedSettingsDialog();
-                if (dlg.getValue() == 1) {
-                    dlg.saveSettings();
-                }
+        bAdv.addActionListener(ignored -> {
+            AdvancedSettingsDialog dlg = new AdvancedSettingsDialog();
+            if (dlg.getValue() == 1) {
+                dlg.saveSettings();
             }
         });
-        panel.add(bAdv, GBC.eol().insets(0, 5, 0, 0).anchor(GBC.EAST));
+        panel.add(bAdv, GBC.eol().insets(0, 5, 0, 0).anchor(GridBagConstraints.EAST));
 
         setupDialog();
@@ -69 +71 @@
     }
 
+    /**
+     * Get the specified max width/diameter
+     * @return The maximum width/diameter for rectangles/circles
+     */
     public final double width() {
         try {
@@ -77 +83 @@
     }
 
+    /**
+     * Get the step length for drawing rectangular buildings
+     * @return The discrete steps to increase rectanglular building sizes
+     */
     public double lenstep() {
         try {
@@ -85 +95 @@
     }
 
+    /**
+     * Check if the user wants to use addresses from underlying nodes
+     * @return {@code true} if the user wants to use addresses
+     */
     public final boolean useAddr() {
         return caddr.isSelected();
     }
 
+    /**
+     * Save the settings for this dialog
+     */
     public final void saveSettings() {
         if (circleRadio.isSelected()) {

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/DrawBuildingAction.java

@@ -39 +39 @@
 import java.util.LinkedList;
 import java.util.Map;
+import java.util.Objects;
 
 import static org.openstreetmap.josm.plugins.buildings_tools.BuildingsToolsPlugin.latlon2eastNorth;
@@ -50 +51 @@
         KeyPressReleaseListener, ModifierExListener {
     private static final long serialVersionUID = -3515263157730927711L;
+    private static final String CROSSHAIR = "crosshair";
+    private static final String BUILDING_STRING = "building";
+    private static final String DRAW_BUILDINGS = marktr("Draw buildings");
     // We need to avoid opening many file descriptors on Linux under Wayland -- see JOSM #21929. This will probably also
     // improve performance, since we aren't creating cursors all the time.
-    private static final Cursor CURSOR_SILO = ImageProvider.getCursor("crosshair", "silo");
-    private static final Cursor CURSOR_BUILDING = ImageProvider.getCursor("crosshair", "building");
+    private static final Cursor CURSOR_SILO = ImageProvider.getCursor(CROSSHAIR, "silo");
+    private static final Cursor CURSOR_BUILDING = ImageProvider.getCursor(CROSSHAIR, BUILDING_STRING);
 
     private enum Mode {
-        None, Drawing, DrawingWidth, DrawingAngFix
+        NONE, DRAWING, DRAWING_WIDTH, DRAWING_ANG_FIX
     }
 
@@ -63 +67 @@
     private Cursor customCursor;
 
-    private Mode mode = Mode.None;
-    private Mode nextMode = Mode.None;
+    private Mode mode = Mode.NONE;
+    private Mode nextMode = Mode.NONE;
 
     private Color selectedColor = Color.red;
@@ -74 +78 @@
     final transient Building building = new Building();
 
-    private final PreferenceChangedListener shapeChangeListener = event -> updCursor();
+    private final transient PreferenceChangedListener shapeChangeListener = event -> updCursor();
 
     /**
@@ -80 +84 @@
      */
     public DrawBuildingAction() {
-        super(tr("Draw buildings"), "building", tr("Draw buildings"),
+        super(tr(DRAW_BUILDINGS), BUILDING_STRING, tr(DRAW_BUILDINGS),
                 Shortcut.registerShortcut("mapmode:buildings",
-                        tr("Mode: {0}", tr("Draw buildings")),
+                        tr("Mode: {0}", tr(DRAW_BUILDINGS)),
                         KeyEvent.VK_B, Shortcut.DIRECT),
                 // Set super.cursor to crosshair without overlay because super.cursor is final,
                 // but we use two different cursors with overlays for rectangular and circular buildings
                 // the actual cursor is drawn in enterMode()
-                ImageProvider.getCursor("crosshair", null));
-
-        cursorJoinNode = ImageProvider.getCursor("crosshair", "joinnode");
-        cursorJoinWay = ImageProvider.getCursor("crosshair", "joinway");
+                ImageProvider.getCursor(CROSSHAIR, null));
+
+        cursorJoinNode = ImageProvider.getCursor(CROSSHAIR, "joinnode");
+        cursorJoinWay = ImageProvider.getCursor(CROSSHAIR, "joinway");
     }
 
@@ -160 +164 @@
         Config.getPref().removeKeyPreferenceChangeListener("buildings_tool.shape", shapeChangeListener);
 
-        if (mode != Mode.None)
+        if (mode != Mode.NONE)
             map.mapView.repaint();
-        mode = Mode.None;
+        mode = Mode.NONE;
     }
 
@@ -169 +173 @@
      */
     public final void cancelDrawing() {
-        mode = Mode.None;
+        mode = Mode.NONE;
         MapFrame map = MainApplication.getMap();
         if (map == null || map.mapView == null)
@@ -188 +192 @@
             processMouseEvent(null);
             updCursor();
-            if (mode != Mode.None)
+            if (mode != Mode.NONE)
                 MainApplication.getMap().mapView.repaint();
         }
@@ -196 +200 @@
     public void doKeyPressed(KeyEvent e) {
         if (e.getKeyCode() == KeyEvent.VK_ESCAPE) {
-            if (mode != Mode.None)
+            if (mode != Mode.NONE)
                 e.consume();
 
@@ -206 +210 @@
         }
         e.consume();
-        switch (ToolSettings.getShape()) {
-            case CIRCLE:
-                ToolSettings.saveShape(ToolSettings.Shape.RECTANGLE);
-                break;
-            case RECTANGLE:
-                ToolSettings.saveShape(ToolSettings.Shape.CIRCLE);
+        ToolSettings.Shape shape = ToolSettings.getShape();
+        if (Objects.requireNonNull(shape) == ToolSettings.Shape.CIRCLE) {
+            ToolSettings.saveShape(ToolSettings.Shape.RECTANGLE);
+        } else if (shape == ToolSettings.Shape.RECTANGLE) {
+            ToolSettings.saveShape(ToolSettings.Shape.CIRCLE);
+        } else {
+            throw new IllegalStateException("Unknown shape " + shape);
         }
     }
@@ -227 +232 @@
             IWaySegment<Node, Way> ws = MainApplication.getMap().mapView.getNearestWaySegment(mousePos,
                     OsmPrimitive::isSelectable);
-            if (ws != null && ws.getWay().get("building") != null) {
+            if (ws != null && ws.getWay().get(BUILDING_STRING) != null) {
                 EastNorth p1 = latlon2eastNorth(ws.getFirstNode());
                 EastNorth p2 = latlon2eastNorth(ws.getSecondNode());
@@ -250 +255 @@
             building.setPlaceRect(p);
             if (Math.abs(building.getLength()) < MIN_LEN_WIDTH)
-                return Mode.Drawing;
-            return shift ? Mode.DrawingAngFix : Mode.None;
+                return Mode.DRAWING;
+            return shift ? Mode.DRAWING_ANG_FIX : Mode.NONE;
         } else if (ToolSettings.Shape.CIRCLE == ToolSettings.getShape()) {
-            if (ToolSettings.getWidth() != 0) {
-                building.setPlaceCircle(p, ToolSettings.getWidth(), shift);
-            } else {
-                building.setPlace(p, ToolSettings.getWidth(), ToolSettings.getLenStep(), shift);
-            }
-            if (building.getLength() < MIN_LEN_WIDTH)
-                return Mode.Drawing;
-            MainApplication.getMap().statusLine.setDist(building.getLength());
-            return Mode.None;
+            return modeDrawingCircle(p);
         } else {
             building.setPlace(p, ToolSettings.getWidth(), ToolSettings.getLenStep(), shift);
             if (building.getLength() < MIN_LEN_WIDTH)
-                return Mode.Drawing;
+                return Mode.DRAWING;
             MainApplication.getMap().statusLine.setDist(building.getLength());
-            return ToolSettings.getWidth() == 0 ? Mode.DrawingWidth : Mode.None;
-        }
+            return ToolSettings.getWidth() == 0 ? Mode.DRAWING_WIDTH : Mode.NONE;
+        }
+    }
+
+    private Mode modeDrawingCircle(EastNorth p) {
+        if (ToolSettings.getWidth() != 0) {
+            building.setPlaceCircle(p, ToolSettings.getWidth(), shift);
+        } else {
+            building.setPlace(p, ToolSettings.getWidth(), ToolSettings.getLenStep(), shift);
+        }
+        if (building.getLength() < MIN_LEN_WIDTH)
+            return Mode.DRAWING;
+        MainApplication.getMap().statusLine.setDist(building.getLength());
+        return Mode.NONE;
     }
 
@@ -275 +284 @@
         double width = Math.abs(building.getWidth());
         MainApplication.getMap().statusLine.setDist(width);
-        return width < MIN_LEN_WIDTH ? Mode.DrawingWidth : Mode.None;
+        return width < MIN_LEN_WIDTH ? Mode.DRAWING_WIDTH : Mode.NONE;
     }
 
     private Mode modeDrawingAngFix() {
         building.angFix(getEastNorth());
-        return Mode.None;
+        return Mode.NONE;
     }
 
@@ -288 +297 @@
             updateKeyModifiers(e);
         }
-        if (mode == Mode.None) {
-            nextMode = Mode.None;
-            return;
-        }
-
-        if (mode == Mode.Drawing) {
+        if (mode == Mode.NONE) {
+            nextMode = Mode.NONE;
+            return;
+        }
+
+        if (mode == Mode.DRAWING) {
             nextMode = modeDrawing();
-        } else if (mode == Mode.DrawingWidth) {
+        } else if (mode == Mode.DRAWING_WIDTH) {
             nextMode = modeDrawingWidth();
-        } else if (mode == Mode.DrawingAngFix) {
+        } else if (mode == Mode.DRAWING_ANG_FIX) {
             nextMode = modeDrawingAngFix();
         } else {
@@ -306 +315 @@
     @Override
     public void paint(Graphics2D g, MapView mv, Bounds bbox) {
-        if (mode == Mode.None || building.getLength() == 0) {
+        if (mode == Mode.NONE || building.getLength() == 0) {
             return;
         }
@@ -324 +333 @@
         EastNorth en = getEastNorth();
         building.setBase(en);
-        mode = Mode.Drawing;
+        mode = Mode.DRAWING;
         updateStatusLine();
     }
@@ -330 +339 @@
     private void drawingAdvance(MouseEvent e) {
         processMouseEvent(e);
-        if (this.mode != Mode.None && this.nextMode == Mode.None) {
+        if (this.mode != Mode.NONE && this.nextMode == Mode.NONE) {
             drawingFinish();
         } else {
@@ -346 +355 @@
                 w = building.createRectangle(ctrl);
             }
-            if (w != null) {
-                if (!alt || ToolSettings.isUsingAddr())
-                    for (Map.Entry<String, String> kv : ToolSettings.getTags().entrySet()) {
-                        w.put(kv.getKey(), kv.getValue());
-                    }
-                if (ToolSettings.isUsingAddr())
-                    showAddrDialog(w);
-                if (ToolSettings.isAutoSelect()
-                        && (getLayerManager().getEditDataSet().getSelected().isEmpty() || shift ||
-                            ToolSettings.isAutoSelectReplaceSelection())) {
-                    getLayerManager().getEditDataSet().setSelected(w);
+            drawingFinish(w);
+        }
+        cancelDrawing();
+    }
+
+    private void drawingFinish(Way w) {
+        if (w != null) {
+            if (!alt || ToolSettings.isUsingAddr())
+                for (Map.Entry<String, String> kv : ToolSettings.getTags().entrySet()) {
+                    w.put(kv.getKey(), kv.getValue());
                 }
-            }
-        }
-        cancelDrawing();
+            if (ToolSettings.isUsingAddr())
+                showAddrDialog(w);
+            if (ToolSettings.isAutoSelect()
+                    && (getLayerManager().getEditDataSet().getSelected().isEmpty() || shift ||
+                    ToolSettings.isAutoSelectReplaceSelection())) {
+                getLayerManager().getEditDataSet().setSelected(w);
+            }
+        }
     }
 
@@ -372 +385 @@
         requestFocusInMapView();
 
-        if (mode == Mode.None)
+        if (mode == Mode.NONE)
             drawingStart(e);
     }
@@ -380 +393 @@
         processMouseEvent(e);
         updCursor();
-        if (mode != Mode.None)
+        if (mode != Mode.NONE)
             MainApplication.getMap().mapView.repaint();
     }
@@ -399 +412 @@
         }
 
-        if (mode == Mode.Drawing && !dragged)
-            return;
-        if (mode == Mode.None)
+        if (mode == Mode.DRAWING && !dragged)
+            return;
+        if (mode == Mode.NONE)
             return;
 
@@ -418 +431 @@
             } else {
                 Way w = MainApplication.getMap().mapView.getNearestWay(mousePos, OsmPrimitive::isSelectable);
-                if (w != null && w.get("building") != null) {
+                if (w != null && w.get(BUILDING_STRING) != null) {
                     setCursor(cursorJoinWay);
                     return;
@@ -437 +450 @@
         processMouseEvent(e);
         updCursor();
-        if (mode != Mode.None)
+        if (mode != Mode.NONE)
             MainApplication.getMap().mapView.repaint();
     }
@@ -443 +456 @@
     @Override
     public String getModeHelpText() {
-        if (mode == Mode.None)
+        if (mode == Mode.NONE)
             return tr("Point on the corner of the building to start drawing");
-        if (mode == Mode.Drawing)
+        if (mode == Mode.DRAWING)
             return tr("Point on opposite end of the building");
-        if (mode == Mode.DrawingWidth)
+        if (mode == Mode.DRAWING_WIDTH)
             return tr("Set width of the building");
         return "";

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/MergeAddrPointsAction.java

@@ -27 +27 @@
 import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.data.UndoRedoHandler;
+import org.openstreetmap.josm.data.osm.IPrimitive;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -43 +44 @@
 public class MergeAddrPointsAction extends JosmAction {
 
+    private static class MultiConflict {
+        private final int multi;
+        private final int conflicts;
+        public MultiConflict(int multi, int conflict) {
+            this.multi = multi;
+            this.conflicts = conflict;
+        }
+    }
+
     /**
      * Merge the address point with the building
@@ -66 +76 @@
         List<Node> addrNodes = new LinkedList<>();
         List<Way> buildings = new LinkedList<>();
+        if (!generateAddressesAndBuildings(selection, addrNodes, buildings)) {
+            return;
+        }
+
+        Set<Way> overlappingWays = removeNodesInMoreThanOneBuilding(addrNodes, buildings);
+
+        List<Command> cmds = new LinkedList<>();
+        List<Pair<Node, Way>> replaced = new ArrayList<>();
+        Set<Relation> modifiedRelations = new HashSet<>();
+
+        MultiConflict multiConflict = parseBuildingWays(cmds, replaced, modifiedRelations, buildings, addrNodes);
+        parseRelations(cmds, replaced, modifiedRelations);
+        if (!replaced.isEmpty()) {
+            final Command deleteCommand = DeleteCommand.delete(replaced.stream().map(p -> p.a).collect(Collectors.toList()));
+            if (deleteCommand != null) {
+                cmds.add(deleteCommand);
+            }
+        }
+
+        generateFinalMessages(cmds, overlappingWays, multiConflict);
+        if (!cmds.isEmpty())
+            UndoRedoHandler.getInstance().add(new SequenceCommand("Merge addresses", cmds));
+    }
+
+    /**
+     * Find the addresses and buildings from the selection
+     * @param selection The selection to look through
+     * @param addrNodes The collection to add address nodes to
+     * @param buildings The collection to add buildings to
+     * @return {@code true} if we can continue on
+     */
+    private static boolean generateAddressesAndBuildings(Collection<OsmPrimitive> selection, List<Node> addrNodes, List<Way> buildings) {
         for (OsmPrimitive p : selection) {
-            if (p.getType() == OsmPrimitiveType.NODE) {
-                boolean refsOK = true;
-                for (OsmPrimitive r : p.getReferrers()) {
-                    if (r.getType() == OsmPrimitiveType.WAY) {
-                        // Don't use nodes if they're referenced by ways
-                        refsOK = false;
-                        break;
-                    }
-                }
-                if (!refsOK)
-                    continue;
+            // Don't use nodes if they're referenced by ways
+            if (p.getType() == OsmPrimitiveType.NODE
+                    && p.getReferrers().stream().map(IPrimitive::getType).noneMatch(OsmPrimitiveType.WAY::equals)) {
                 for (String key : p.getKeys().keySet()) {
                     if (key.startsWith("addr:")) {
@@ -90 +124 @@
             new Notification(tr("No address nodes found in the selection"))
                     .setIcon(JOptionPane.ERROR_MESSAGE).show();
-            return;
+            return false;
         }
         if (buildings.isEmpty()) {
             new Notification(tr("No building ways found in the selection"))
                     .setIcon(JOptionPane.ERROR_MESSAGE).show();
-            return;
-        }
-
+            return false;
+        }
+        return true;
+    }
+
+    /**
+     * Remove nodes that are in more than one building
+     * @param addrNodes The address nodes to look through
+     * @param buildings The buildings to look through
+     * @return The overlapping ways
+     */
+    private static Set<Way> removeNodesInMoreThanOneBuilding(List<Node> addrNodes, List<Way> buildings) {
         // find nodes covered by more than one building, see #17625
         Map<Node, Way> nodeToWayMap = new HashMap<>();
@@ -113 +156 @@
         }
         buildings.removeAll(overlappingWays);
-
-        List<Command> cmds = new LinkedList<>();
+        return overlappingWays;
+    }
+
+    private static MultiConflict parseBuildingWays(List<Command> cmds, List<Pair<Node, Way>> replaced,
+                                          Set<Relation> modifiedRelations, List<Way> buildings,
+                                          List<Node> addrNodes) {
         int multi = 0;
         int conflicts = 0;
-        List<Pair<Node, Way>> replaced = new ArrayList<>();
-        Set<Relation> modifiedRelations = new HashSet<>();
         for (Way w : buildings) {
             Node mergeNode = null;
@@ -137 +182 @@
                 continue;
             if (mergeNode != null) {
-                boolean hasConflicts = false;
-                Map<String, String> tags = new HashMap<>();
-                for (Map.Entry<String, String> entry : mergeNode.getKeys().entrySet()) {
-                    String newValue = entry.getValue();
-                    if (newValue == null)
-                        continue;
-                    String oldValue = w.getKeys().get(entry.getKey());
-                    if (!newValue.equals(oldValue)) {
-                        if (oldValue == null) {
-                            tags.put(entry.getKey(), newValue);
-                        } else
-                            hasConflicts = true;
-                    }
-                }
-                if (hasConflicts)
-                    conflicts++;
-                if (!tags.isEmpty())
-                    cmds.add(new ChangePropertyCommand(Collections.singleton(w), tags));
-                if (!hasConflicts) {
-                    replaced.add(Pair.create(mergeNode, w));
-                    modifiedRelations.addAll(mergeNode.referrers(Relation.class).collect(Collectors.toList()));
-                }
-            }
-        }
-
+                conflicts += checkForConflicts(cmds, replaced, modifiedRelations, w, mergeNode);
+            }
+        }
+        return new MultiConflict(multi, conflicts);
+    }
+
+    private static int checkForConflicts(List<Command> cmds, List<Pair<Node, Way>> replaced,
+                                         Set<Relation> modifiedRelations, Way w, Node mergeNode) {
+        boolean hasConflicts = false;
+        int conflicts = 0;
+        Map<String, String> tags = new HashMap<>();
+        for (Map.Entry<String, String> entry : mergeNode.getKeys().entrySet()) {
+            String newValue = entry.getValue();
+            if (newValue == null)
+                continue;
+            String oldValue = w.getKeys().get(entry.getKey());
+            if (!newValue.equals(oldValue)) {
+                if (oldValue == null) {
+                    tags.put(entry.getKey(), newValue);
+                } else
+                    hasConflicts = true;
+            }
+        }
+        if (hasConflicts)
+            conflicts++;
+        if (!tags.isEmpty())
+            cmds.add(new ChangePropertyCommand(Collections.singleton(w), tags));
+        if (!hasConflicts) {
+            replaced.add(Pair.create(mergeNode, w));
+            modifiedRelations.addAll(mergeNode.referrers(Relation.class).collect(Collectors.toList()));
+        }
+        return conflicts;
+    }
+
+    private static void parseRelations(List<Command> cmds, List<Pair<Node, Way>> replaced, Set<Relation> modifiedRelations) {
         for (Relation r : modifiedRelations) {
             List<RelationMember> members = new ArrayList<>(r.getMembers());
@@ -178 +233 @@
             }
         }
-        if (!replaced.isEmpty()) {
-            final Command deleteCommand = DeleteCommand.delete(replaced.stream().map(p -> p.a).collect(Collectors.toList()));
-            if (deleteCommand != null) {
-                cmds.add(deleteCommand);
-            }
-        }
-
+    }
+
+    private static void generateFinalMessages(List<Command> cmds, Set<Way> overlappingWays, MultiConflict multiConflict) {
+        final int multi = multiConflict.multi;
+        final int conflicts = multiConflict.conflicts;
         if (multi != 0)
             new Notification(trn("There is {0} building with multiple address nodes inside",
@@ -191 +244 @@
         if (conflicts != 0)
             new Notification(trn("There is {0} building with address conflicts",
-                            "There are {0} buildings with address conflicts", conflicts, conflicts))
+                    "There are {0} buildings with address conflicts", conflicts, conflicts))
                     .setIcon(JOptionPane.WARNING_MESSAGE).show();
         if (!overlappingWays.isEmpty())
@@ -199 +252 @@
             new Notification(tr("No address nodes inside buildings found"))
                     .setIcon(JOptionPane.INFORMATION_MESSAGE).show();
-        if (!cmds.isEmpty())
-            UndoRedoHandler.getInstance().add(new SequenceCommand("Merge addresses", cmds));
     }
 

applications/editors/josm/plugins/buildings_tools/src/org/openstreetmap/josm/plugins/buildings_tools/MyDialog.java

@@ -5 +5 @@
 
 import java.awt.Component;
+import java.awt.GridBagConstraints;
 import java.awt.GridBagLayout;
 import java.awt.Insets;
@@ -15 +16 @@
 import org.openstreetmap.josm.tools.GBC;
 
+/**
+ * A customized dialog for buildings tools
+ */
 public abstract class MyDialog extends ExtendedDialog {
     private static final String[] BUTTON_TEXTS = new String[] {tr("OK"), tr("Cancel")};
@@ -25 +29 @@
         panel.add(label, GBC.std());
         label.setLabelFor(c);
-        panel.add(c, GBC.eol().fill(GBC.HORIZONTAL));
+        panel.add(c, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
     }
 
-    public MyDialog(String title) {
+    /**
+     * Create a new dialog
+     * @param title The title for the dialog
+     */
+    protected MyDialog(String title) {
         super(MainApplication.getMainFrame(), title, BUTTON_TEXTS, true);
         contentInsets = new Insets(15, 15, 5, 15);