
Hi WanMil, I agree that also Coord.equals() is a method which has to be handled with great care. The meaning of "p1.equals(p2) == true" is that both points have coordinates that were rounded to equal map unit values. The original OSM points can have a distance of > 2m, on the other hand, two points with a distance < 0.2m can have different map unit coordinates. I know one case where the Coord.equals() has a meaning: When creating the road network, we must make sure that a RouteArc connects two points that have different coordinates. (high prec branch r2861 still contains an error here) I think equals() should be avoided in all algos that try to calculate something. problem is that we use Coord.equals() whenever we use collections like Map<Coord,xyz> or even List<Coord> in combinations with List.contains(). Most of these usages are candidates for errors when input data allows that p1.equals(p2) == true and p1 != p2. The higher precision values in the high prec branch are no solution for that, they just make it less likely to happen. Maybe a good way to find potential errors in the code is to create test data that contains these special cases ? Gerd
Date: Thu, 5 Dec 2013 21:59:42 +0100 From: wmgcnfg@web.de To: mkgmap-dev@lists.mkgmap.org.uk Subject: Re: [mkgmap-dev] Way.isClosed()
Hi Gerd,
I've gone through all calls of isClosed(). I am quite sure all in all calls it should be checked for identity.
But there are some other changes that need to be done (not complete!!): SeaGenerator beginMap (line 770) need to be IdentityHashMap
BoundaryRelation line 387 should be an IdentityHashMap? line 474+477: Coord.equals(..) maybe check for identity?
MultipolgyonRelation line 468 should be an IdentityHashMap? line 553+556: Coord.equals(..) maybe check for identity?
I guess there are a lot of other places where Coord.equals is used instead of identity.
WanMil
Hi WanMil,
Before fixing that all calls to this method need to be checked if they rely on the current behaviour.
I agree. The problem is that the method is used in rather complex algos, so it's difficult for me to answer this question, esp. when we also have to consider the changes in the high prec branch.
Gerd
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev