Hi Gerd,<br><br>I was completely wrong about the compareTo. Thanks for pointing that out. <br><br>I agree the compareTo method should be removed. TableA should really be using an IdentityLinkedHashMap, but this doesn't exist in the standard jdk. <br><br>Thanks,<br>Brian<br><div class="gmail_quote">On Thu, Nov 6, 2014 at 9:45 AM Gerd Petermann <<a href="mailto:gpetermann_muenchen@hotmail.com">gpetermann_muenchen@hotmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div dir="ltr">Hi Brian,<br><br>I read again your posts. I think you got something wrong.<br>I think we allow to create multiple instances of RoadDef<br>having the same values in the id field and in the name field.<br>This happens when a style adds the same OSM way <br>with different routing attributes, e.g. one way for bicycles<br>and one for cars.<br>A public available style that does this is Minkos:<br><a href="http://mijndev.openstreetmap.nl/%7Eligfietser/openfietsmap/Scripts/" target="_blank">http://mijndev.openstreetmap.nl/%7Eligfietser/openfietsmap/Scripts/</a><br><br>I think your patch disables this feature.</div></div><div><div dir="ltr"><br><br>Gerd<br><br></div></div><div><div dir="ltr"><div><hr>From: <a href="mailto:brianegge@gmail.com" target="_blank">brianegge@gmail.com</a><br>Date: Thu, 6 Nov 2014 06:34:43 -0500<br>To: <a href="mailto:mkgmap-dev@lists.mkgmap.org.uk" target="_blank">mkgmap-dev@lists.mkgmap.org.uk</a><br>Subject: Re: [mkgmap-dev] RoadDef patch<br><br></div></div></div><div><div dir="ltr"><div><div>Hi Gerd,</div><div><br></div><div>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. </div><div><br></div><div>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. </div><div><br></div><div>The sole purpose of the Hash seems to be preventing multiple, duplicate arcs from being created on the same RouteNode. </div><div><br></div><div>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. </div><div><br></div><div>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. </div><div><br></div><div>Brian</div><div><br></div><div><br></div><div>When RoadDefs are created in NETFileReader, a unique id is generated for each RoadDef. <span style="white-space:pre-wrap"> </span></div><br><div><blockquote><div>On Nov 6, 2014, at 3:34 AM, Gerd Petermann <<a href="mailto:gpetermann_muenchen@hotmail.com" target="_blank">gpetermann_muenchen@hotmail.com</a>> wrote:</div><br><div><div dir="ltr" style="font-family:Calibri;font-size:16px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Hi Brian,<br><br>thanks for the patch. If I got it right, the compareTo() methid is obsolete<br>as we do no longer sort RoadDef instances anywhere. So,<br>it would be easier to remove the Comparable attribute from the class<br>public class RoadDef /*implements Comparable<RoadDef>*/ {<br>and remove the compareTo() method.<br><br>You are right regarding the hashCode() method, but I think<br>we should not use the fields id + name to implement it.<br>A style may add the same OSM way as a routable line multiple times,<br>in that case these two fields are equal.<br><br>A typical map has less than 100.000 RoadDef instances, so I see no problem<br>to add an int (or long) field like roadId which is filled with a unique value<br>and use that in hashCode().<br>What do you think?<br><br>Gerd<br><br><div>From: <a href="mailto:brianegge@gmail.com" target="_blank">brianegge@gmail.com</a><br>Date: Wed, 5 Nov 2014 22:28:38 -0500<br>To: <a href="mailto:mkgmap-dev@lists.mkgmap.org.uk" target="_blank">mkgmap-dev@lists.mkgmap.org.uk</a><br>Subject: [mkgmap-dev] RoadDef patch<br><br><pre>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. <br> <br>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. <br> <br></pre><br>Brian<br>_______________________________________________ mkgmap-dev mailing list<span> </span><a href="mailto:mkgmap-dev@lists.mkgmap.org.uk" target="_blank">mkgmap-dev@lists.mkgmap.org.uk</a><span> </span><a href="http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev" target="_blank">http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev</a></div></div><span style="font-family:Calibri;font-size:16px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;display:inline!important">_______________________________________________</span><br style="font-family:Calibri;font-size:16px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Calibri;font-size:16px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;display:inline!important">mkgmap-dev mailing list</span><br style="font-family:Calibri;font-size:16px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><a href="mailto:mkgmap-dev@lists.mkgmap.org.uk" style="font-family:Calibri;font-size:16px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" target="_blank">mkgmap-dev@lists.mkgmap.org.uk</a><br style="font-family:Calibri;font-size:16px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><a href="http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev" style="font-family:Calibri;font-size:16px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" target="_blank">http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev</a></div></blockquote></div><br><br>_______________________________________________
mkgmap-dev mailing list
<a href="mailto:mkgmap-dev@lists.mkgmap.org.uk" target="_blank">mkgmap-dev@lists.mkgmap.org.uk</a>
<a href="http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev" target="_blank">http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev</a></div></div></div>
______________________________<u></u>_________________<br>
mkgmap-dev mailing list<br>
<a href="mailto:mkgmap-dev@lists.mkgmap.org.uk" target="_blank">mkgmap-dev@lists.mkgmap.org.uk</a><br>
<a href="http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev" target="_blank">http://www.mkgmap.org.uk/<u></u>mailman/listinfo/mkgmap-dev</a></blockquote></div>