Changeset 5608 in josm

Timestamp:

2012-11-28T23:47:46+01:00 (12 years ago)

Author:

stoecker

Message:

don't hardcode created_by for DuplicateRelation validator test, javadoc fixes

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/upload/DiscardTagsHook.java (modified) (1 diff)
• data/osm/OsmPrimitive.java (modified) (6 diffs)
• data/validation/tests/DuplicateRelation.java (modified) (10 diffs)

trunk/src/org/openstreetmap/josm/actions/upload/DiscardTagsHook.java

@@ -21 +21 @@
 public class DiscardTagsHook implements UploadHook {
 
+    @Override
     public boolean checkUpload(APIDataSet apiDataSet) {
         List<OsmPrimitive> objectsToUpload = apiDataSet.getPrimitives();

trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java

@@ -128 +128 @@
      * If <code>list</code> is null, replies an empty set.
      *
-     * @param <T>
-     * @param list  the original collection
+     * @param <T> type of data (must be one of the {@link OsmPrimitive} types
+     * @param set  the original collection
      * @param type the type to filter for
      * @return the sub-set of OSM primitives of type <code>type</code>
@@ -621 +621 @@
     public static Collection<String> getUninterestingKeys() {
         if (uninteresting == null) {
-            uninteresting = Main.pref.getCollection("tags.uninteresting",
-                    Arrays.asList("source", "source_ref", "source:", "note", "comment",
-                            "converted_by", "created_by", "watch", "watch:", "fixme", "FIXME",
-                            "description", "attribution"));
+            LinkedList<String> l = new LinkedList<String>(Arrays.asList(
+                "source", "source_ref", "source:", "note", "comment",
+                "converted_by", "watch", "watch:", "fixme", "FIXME",
+                "description", "attribution"));
+            l.addAll(getDiscardableKeys());
+            uninteresting = Main.pref.getCollection("tags.uninteresting", l);
         }
         return uninteresting;
@@ -967 +969 @@
     /*-----------------
      * OTHER METHODS
-     *----------------/
+     *----------------*/
 
     /**
@@ -1075 +1077 @@
     /**
      * Loads (clone) this primitive from provided PrimitiveData
-     * @param data
+     * @param data The object which should be cloned
      */
     public void load(PrimitiveData data) {
@@ -1091 +1093 @@
     /**
      * Save parameters of this primitive to the transport object
-     * @return
+     * @return The saved object data
      */
     public abstract PrimitiveData save();
 
+    /**
+     * Save common parameters of primitives to the transport object
+     * @param data The object to save the data into
+     */
     protected void saveCommonAttributes(PrimitiveData data) {
         data.setId(id);
@@ -1108 +1114 @@
     }
 
+    /**
+     * Fetch the bounding box of the primitive
+     * @return Bounding box of the object
+     */
     public abstract BBox getBBox();
 

trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java

@@ -28 +28 @@
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.tools.MultiMap;
+
 /**
  * Tests if there are duplicate relations
  */
-public class DuplicateRelation extends Test
-{
-
+public class DuplicateRelation extends Test {
+
+    /**
+     * Class to store one relation members and information about it
+     */
     public static class RelMember {
+        /** Role of the relation member */
         private String role;
+
+        /** Type of the relation member */
         private OsmPrimitiveType type;
+
+        /** Tags of the relation member */
         private Map<String, String> tags;
+
+        /** Coordinates of the relation member */
         private List<LatLon> coor;
+
+        /** ID of the relation member in case it is a {@link Relation} */
         private long rel_id;
 
@@ -53 +65 @@
         }
 
+        /** Extract and store relation information based on the relation member
+         * @param src The relation member to store information about
+         */
         public RelMember(RelationMember src) {
-            role=src.getRole();
-            type=src.getType();
-            rel_id=0;
-            coor=new ArrayList<LatLon>();
+            role = src.getRole();
+            type = src.getType();
+            rel_id = 0;
+            coor = new ArrayList<LatLon>();
 
             if (src.isNode()) {
-                Node r=src.getNode();
-                tags=r.getKeys();
-                coor=new ArrayList<LatLon>(1);
+                Node r = src.getNode();
+                tags = r.getKeys();
+                coor = new ArrayList<LatLon>(1);
                 coor.add(r.getCoor());
             }
             if (src.isWay()) {
-                Way r=src.getWay();
-                tags=r.getKeys();
+                Way r = src.getWay();
+                tags = r.getKeys();
                 List<Node> wNodes = r.getNodes();
-                coor=new ArrayList<LatLon>(wNodes.size());
-                for (int i=0;i<wNodes.size();i++) {
+                coor = new ArrayList<LatLon>(wNodes.size());
+                for (int i = 0; i < wNodes.size(); i++) {
                     coor.add(wNodes.get(i).getCoor());
                 }
             }
             if (src.isRelation()) {
-                Relation r=src.getRelation();
-                tags=r.getKeys();
-                rel_id=r.getId();
-                coor=new ArrayList<LatLon>();
-            }
-        }
-    }
-
+                Relation r = src.getRelation();
+                tags = r.getKeys();
+                rel_id = r.getId();
+                coor = new ArrayList<LatLon>();
+            }
+        }
+    }
+
+    /**
+     * Class to store relation members
+     */
     private class RelationMembers {
+        /** List of member objects of the relation */
         public List<RelMember> members;
-        public RelationMembers(List<RelationMember> _members) {
-            members=new ArrayList<RelMember>(_members.size());
-            for (int i=0;i<_members.size();i++) {
-                members.add(new RelMember(_members.get(i)));
-            }
-        }
+
+        /** Store relation information
+         * @param members The list of relation members
+         */
+        public RelationMembers(List<RelationMember> members) {
+            this.members = new ArrayList<RelMember>(members.size());
+            for (int i = 0; i < members.size(); i++) {
+                this.members.add(new RelMember(members.get(i)));
+            }
+        }
+
         @Override
         public int hashCode() {
             return members.hashCode();
         }
+
         @Override
         public boolean equals(Object obj) {
@@ -103 +128 @@
     }
 
+    /**
+     * Class to store relation data (keys are usually cleanup and may not be equal to original relation)
+     */
     private class RelationPair {
+        /** Member objects of the relation */
         public RelationMembers members;
+        /** Tags of the relation */
         public Map<String, String> keys;
-        public RelationPair(List<RelationMember> _members,Map<String, String> _keys) {
-            members=new RelationMembers(_members);
-            keys=_keys;
-        }
+
+        /** Store relation information
+         * @param members The list of relation members
+         * @param keys The set of tags of the relation
+         */
+        public RelationPair(List<RelationMember> members, Map<String, String> keys) {
+            this.members = new RelationMembers(members);
+            this.keys = keys;
+        }
+
         @Override
         public int hashCode() {
             return members.hashCode()+keys.hashCode();
         }
+
         @Override
         public boolean equals(Object obj) {
@@ -122 +159 @@
     }
 
+    /** Code number of completely duplicated relation error */
     protected static final int DUPLICATE_RELATION = 1901;
+
+    /** Code number of relation with same members error */
     protected static final int SAME_RELATION = 1902;
 
@@ -131 +171 @@
     MultiMap<List<RelationMember>, OsmPrimitive> relations_nokeys;
 
-    /**
-     * Constructor
-     */
-    public DuplicateRelation()
-    {
+    /** List of keys without useful information */
+    Collection<String> ignoreKeys = new HashSet<String>(OsmPrimitive.getUninterestingKeys());
+
+    /**
+     * Default constructor
+     */
+    public DuplicateRelation() {
         super(tr("Duplicated relations"),
                 tr("This test checks that there are no relations with same tags and same members with same roles."));
     }
 
-
-    @Override
-    public void startTest(ProgressMonitor monitor)
-    {
+    @Override
+    public void startTest(ProgressMonitor monitor) {
         super.startTest(monitor);
         relations = new MultiMap<RelationPair, OsmPrimitive>(1000);
@@ -150 +190 @@
 
     @Override
-    public void endTest()
-    {
+    public void endTest() {
         super.endTest();
-        for(LinkedHashSet<OsmPrimitive> duplicated : relations.values() )
-        {
-            if( duplicated.size() > 1)
-            {
+        for(LinkedHashSet<OsmPrimitive> duplicated : relations.values()) {
+            if (duplicated.size() > 1) {
                 TestError testError = new TestError(this, Severity.ERROR, tr("Duplicated relations"), DUPLICATE_RELATION, duplicated);
                 errors.add( testError );
@@ -162 +199 @@
         }
         relations = null;
-        for(LinkedHashSet<OsmPrimitive> duplicated : relations_nokeys.values() )
-        {
-            if( duplicated.size() > 1)
-            {
+        for(LinkedHashSet<OsmPrimitive> duplicated : relations_nokeys.values()) {
+            if (duplicated.size() > 1) {
                 TestError testError = new TestError(this, Severity.WARNING, tr("Relations with same members"), SAME_RELATION, duplicated);
                 errors.add( testError );
@@ -174 +209 @@
 
     @Override
-    public void visit(Relation r)
-    {
+    public void visit(Relation r) {
         if (!r.isUsable() || r.hasIncompleteMembers())
             return;
-        List<RelationMember> rMembers=r.getMembers();
-        Map<String, String> rkeys=r.getKeys();
-        rkeys.remove("created_by");
-        RelationPair rKey=new RelationPair(rMembers,rkeys);
+        List<RelationMember> rMembers = r.getMembers();
+        Map<String, String> rkeys = r.getKeys();
+        for (String key : ignoreKeys)
+            rkeys.remove(ignoreKeys);
+        RelationPair rKey = new RelationPair(rMembers, rkeys);
         relations.put(rKey, r);
         relations_nokeys.put(rMembers, r);
@@ -188 +223 @@
     /**
      * Fix the error by removing all but one instance of duplicate relations
-     */
-    @Override
-    public Command fixError(TestError testError)
-    {
+     * @param testError The error to fix, must be of type {@link #DUPLICATE_RELATION}
+     */
+    @Override
+    public Command fixError(TestError testError) {
         if (testError.getCode() == SAME_RELATION) return null;
         Collection<? extends OsmPrimitive> sel = testError.getPrimitives();
@@ -253 +288 @@
 
     @Override
-    public boolean isFixable(TestError testError)
-    {
-        if (!(testError.getTester() instanceof DuplicateRelation))
-            return false;
-
-        if (testError.getCode() == SAME_RELATION) return false;
+    public boolean isFixable(TestError testError) {
+        if (!(testError.getTester() instanceof DuplicateRelation)
+            || testError.getCode() == SAME_RELATION) return false;
 
         // We fix it only if there is no more than one relation that is relation member.