[mkgmap-dev] RoadDef patch

Thu Nov 6 16:39:03 GMT 2014

Hi Gerd,

I was completely wrong about the compareTo. Thanks for pointing that out.

I agree the compareTo method should be removed. TableA should really be
using an IdentityLinkedHashMap, but this doesn't exist in the standard jdk.

Thanks,
Brian
On Thu, Nov 6, 2014 at 9:45 AM Gerd Petermann <
gpetermann_muenchen at hotmail.com> wrote:

> Hi Brian,
>
> I read again your posts. I think you got something wrong.
> I think we allow to create multiple instances of RoadDef
> having the same values in the id field and in the name field.
> This happens when a style adds the same OSM way
> with different routing attributes, e.g. one way for bicycles
> and one for cars.
> A public available style that does this is Minkos:
> http://mijndev.openstreetmap.nl/%7Eligfietser/openfietsmap/Scripts/
>
> I think your patch disables this feature.
>
>
> Gerd
>
> ------------------------------
> From: brianegge at gmail.com
> Date: Thu, 6 Nov 2014 06:34:43 -0500
> To: mkgmap-dev at lists.mkgmap.org.uk
> Subject: Re: [mkgmap-dev] RoadDef patch
>
> Hi Gerd,
>
> In Steve’s stack trace it shows RoadMap is being used by a LinkedHashMap
> in TableA. A linked hash map maintains an ordered list of nodes, which is
> the same in every JDK version. You can set a breakpoint in CompareTo and
> run the SimpleRouteTest and see that it’s being used.
>
> The TableA addArc method is trying to only add unique arcs. They
> containsKey method at present will never return true, but it’s clear from
> the code that there are case where it should return true. My change results
> in a slight reduction of the NOD section, which seems to make sense.
>
> The sole purpose of the Hash seems to be preventing multiple, duplicate
> arcs from being created on the same RouteNode.
>
> In MapNode, RoadDef is constructed with the osm id of the way, but when
> constructed from the NETFileReader, the id is the record number, which will
> be unique.
>
> If every RoadDef should be unique, then one should not implement
> equals/hashCode/compareable, and instead use the default implementations
> from Object. However, one would then need to remove the LinkedHashMap from
> TableA and replace it with something like Array.
>
> Brian
>
>
> When RoadDefs are created in NETFileReader, a unique id is generated for
> each RoadDef.
>
> On Nov 6, 2014, at 3:34 AM, Gerd Petermann <
> gpetermann_muenchen at hotmail.com> wrote:
>
> Hi Brian,
>
> thanks for the patch. If I got it right, the compareTo() methid is obsolete
> as we do no longer sort RoadDef instances anywhere. So,
> it would be easier to remove the Comparable attribute from the class
> public class RoadDef /*implements Comparable<RoadDef>*/ {
> and remove the compareTo() method.
>
> You are right regarding the hashCode() method, but I think
> we should not use the fields id + name to implement it.
> A style may add the same OSM way as a routable line multiple times,
> in that case these two fields are equal.
>
> A typical map has less than 100.000 RoadDef instances, so I see no problem
> to add an int (or long) field like roadId which is filled with a unique
> value
> and use that in hashCode().
> What do you think?
>
> Gerd
>
> From: brianegge at gmail.com
> Date: Wed, 5 Nov 2014 22:28:38 -0500
> To: mkgmap-dev at lists.mkgmap.org.uk
> Subject: [mkgmap-dev] RoadDef patch
>
> I’m not sure what the concurrency issue is related to this class, but it seems to have some problems around equality. First, it’s being used in a HashMap, which means both equals and hashCode need to be implemented. Second, the compareTo was using an unimplemented hashCode, which results in random sorting and inability to find two identical objects.
>
> I’m not sure I understand all the details of what RoadDef does, but I’ve fixed up the basic methods in the class to be consistent and added a test case around those methods.
>
>
> Brian
> _______________________________________________ mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>
>
>
> _______________________________________________ mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mkgmap.org.uk/pipermail/mkgmap-dev/attachments/20141106/a26a4659/attachment.html>