[mkgmap-dev] [PATCH v1] Add null handling for Element.copyTags and Element.getTagsWithPrefix

[WanMil]

On 27/02/10 13:23, WanMil wrote:
/**
    * Copy the tags of the other element.  Only to be used internally
    * by subclasses.
    * @param other The other element.  All its tags will be copied to this
    * element.
    */
public void copyTags(Element other) {
        if (other.tags != null)
                tags = other.tags.copy();
}

So the copyTags does not add but replace all tags. But only if the other
element has tags. This is weird. So I think replaceTags would be a
better name and the other.tags==null case should be handled:

Originally it was used to copy tags from an existing element to a
newly created one, so there wasn't really much of a problem.

This is still mostly the case, except in the multipolygon code which
is no doubt why you are bringing this up :)

So yes, your changes appear fine to me as they will not affect the
original cases. Also the comment about only being used in subclasses
is wrong/out of date.

..Steve

Yeah, I use this in the multipolygon code and stumbled over it.

The patch fixes the copyTags method and adds a null check to the 
getTagsWithPrefix method (which might have been bigger problem).

WanMil
Index: src/uk/me/parabola/mkgmap/reader/osm/Element.java
===================================================================
--- src/uk/me/parabola/mkgmap/reader/osm/Element.java   (revision 1585)
+++ src/uk/me/parabola/mkgmap/reader/osm/Element.java   (working copy)
@@ -82,13 +82,15 @@
        }
 
        /**
-        * Copy the tags of the other element.  Only to be used internally
-        * by subclasses.
+        * Copy the tags of the other element which replaces all tags of this 
element.
+        *   
         * @param other The other element.  All its tags will be copied to this
         * element.
         */
        public void copyTags(Element other) {
-               if (other.tags != null)
+               if (other.tags == null)
+                       tags = null;
+               else
                        tags = other.tags.copy();
        }
 
@@ -102,6 +104,9 @@
        }
 
        public Map<String, String> getTagsWithPrefix(String prefix, boolean 
removePrefix) {
+               if (tags == null) 
+                       return Collections.emptyMap();
+               
                return tags.getTagsWithPrefix(prefix, removePrefix);
        }
 
_______________________________________________
mkgmap-dev mailing list
mkgmap-dev@lists.mkgmap.org.uk
http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev