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