Changeset 36217 in osm for applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/ReconstructRouteAction.java

Timestamp:

2024-03-03T10:25:25+01:00 (22 months ago)

Author:

GerdP

Message:

fix #23521: fix some memory leaks

• dispose dialogs
• either avoid to create clones of ways or relations or use setNodes(null) / setMembers(null)
• replaces most ChangeCommand instances by better specialized alternatives
• add some comments
• fix some checkstyle / sonar issues

File:

• applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/ReconstructRouteAction.java (modified) (9 diffs)

applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/ReconstructRouteAction.java

@@ -13 +13 @@
 import javax.swing.AbstractAction;
 
-import org.openstreetmap.josm.command.ChangeCommand;
-import org.openstreetmap.josm.command.Command;
+import org.openstreetmap.josm.command.ChangeMembersCommand;
 import org.openstreetmap.josm.data.UndoRedoHandler;
 import org.openstreetmap.josm.data.coor.EastNorth;
@@ -35 +34 @@
  */
 public class ReconstructRouteAction extends AbstractAction implements ChosenRelationListener {
-    private final ChosenRelation rel;
-
+    private final transient ChosenRelation rel;
+
+    /**
+     * Reconstruct route relation to scheme of public_transport.
+     * @param rel chosen relation
+     */
     public ReconstructRouteAction(ChosenRelation rel) {
         super(tr("Reconstruct route"));
@@ -49 +52 @@
     public void actionPerformed(ActionEvent e) {
         Relation r = rel.get();
-        Relation recRel = new Relation(r);
-        recRel.removeMembersFor(recRel.getMemberPrimitives());
+        List<RelationMember> recMembers = new ArrayList<>();
 
         Map<OsmPrimitive, RelationMember> stopMembers = new LinkedHashMap<>();
@@ -118 +120 @@
                     nIndex += increment) {
                 Node refNode = w.getNode(nIndex);
-                if (PublicTransportHelper.isNodeStop(refNode)) {
-                    if (stopMembers.containsKey(refNode)) {
-                        recRel.addMember(stopMembers.get(refNode));
-                        stopMembers.remove(refNode);
-                        String stopName = PublicTransportHelper.getNameViaStoparea(refNode);
-                        if (stopName == null) {
-                            stopName = "";
+                if (!(PublicTransportHelper.isNodeStop(refNode) && stopMembers.containsKey(refNode))) 
+                    continue;
+                recMembers.add(stopMembers.get(refNode));
+                stopMembers.remove(refNode);
+                String stopName = PublicTransportHelper.getNameViaStoparea(refNode);
+                if (stopName == null) {
+                    stopName = "";
+                }
+                boolean existsPlatform = platformMembers.containsKey(stopName);
+                if (!existsPlatform) {
+                    stopName = ""; // find of the nameless
+                }
+                if (existsPlatform || platformMembers.containsKey(stopName)) {
+                    List<RelationMember> lMember = platformMembers.get(stopName);
+                    if (lMember.size() == 1) {
+                        recMembers.add(lMember.get(0));
+                        lMember.remove(0);
+                    } else {
+                        // choose closest
+                        RelationMember candidat = getClosestPlatform(lMember, refNode);
+                        if (candidat != null) {
+                            recMembers.add(candidat);
+                            lMember.remove(candidat);
                         }
-                        boolean existsPlatform = platformMembers.containsKey(stopName);
-                        if (!existsPlatform) {
-                            stopName = ""; // find of the nameless
-                        }
-                        if (existsPlatform || platformMembers.containsKey(stopName)) {
-                            List<RelationMember> lMember = platformMembers.get(stopName);
-                            if (lMember.size() == 1) {
-                                recRel.addMember(lMember.get(0));
-                                lMember.remove(0);
-                            } else {
-                                // choose closest
-                                RelationMember candidat = getClosestPlatform(lMember, refNode);
-                                if (candidat != null) {
-                                    recRel.addMember(candidat);
-                                    lMember.remove(candidat);
-                                }
-                            }
-                            if (lMember.isEmpty()) {
-                                platformMembers.remove(stopName);
-                            }
-                        }
+                    }
+                    if (lMember.isEmpty()) {
+                        platformMembers.remove(stopName);
                     }
                 }
@@ -153 +153 @@
 
         for (RelationMember stop : stopMembers.values()) {
-            recRel.addMember(stop);
+            recMembers.add(stop);
             String stopName = PublicTransportHelper.getNameViaStoparea(stop);
             boolean existsPlatform = platformMembers.containsKey(stopName);
@@ -162 +162 @@
                 List<RelationMember> lMember = platformMembers.get(stopName);
                 if (lMember.size() == 1) {
-                    recRel.addMember(lMember.get(0));
+                    recMembers.add(lMember.get(0));
                     lMember.remove(0);
                 } else {
@@ -168 +168 @@
                     RelationMember candidat = getClosestPlatform(lMember, stop.getNode());
                     if (candidat != null) {
-                        recRel.addMember(candidat);
+                        recMembers.add(candidat);
                         lMember.remove(candidat);
                     }
@@ -180 +180 @@
         for (List<RelationMember> lPlatforms : platformMembers.values()) {
             for (RelationMember platform : lPlatforms) {
-                recRel.addMember(platform);
+                recMembers.add(platform);
             }
         }
 
         for (RelationMember route : routeMembers) {
-            recRel.addMember(route);
+            recMembers.add(route);
         }
         for (RelationMember wtf : wtfMembers) {
-            recRel.addMember(wtf);
-        }
-        Command command = new ChangeCommand(r, recRel);
-        UndoRedoHandler.getInstance().add(command);
+            recMembers.add(wtf);
+        }
+        UndoRedoHandler.getInstance().add(new ChangeMembersCommand(r, recMembers));
     }
 
     private static final double maxSqrDistBetweenStopAndPlatform = 2000; // ~ 26m
-    private RelationMember getClosestPlatform(List<RelationMember> members, Node stop) {
+    
+    private static RelationMember getClosestPlatform(List<RelationMember> members, Node stop) {
         if (stop == null || members.isEmpty()) return null;
         double maxDist = maxSqrDistBetweenStopAndPlatform;
@@ -229 +229 @@
     }
 
-    private boolean isSuitableRelation(Relation newRelation) {
+    private static boolean isSuitableRelation(Relation newRelation) {
         return !(newRelation == null || !"route".equals(newRelation.get("type")) || newRelation.getMembersCount() == 0);
     }