[Dev] Some things we noted while porting

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[Dev] Some things we noted while porting

Itamar Syn-Hershko
A few more questions / comments I have:

1. Double equality checks all throughout the project - why are you using the == operator?

2. In TestShapesGet.getContext, the actual test to run is being selected randomly - why??

_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com
Reply | Threaded
Open this post in threaded view
|

Re: [Dev] Some things we noted while porting

dsmiley

On Apr 28, 2012, at 7:51 PM, Itamar Syn-Hershko wrote:

> A few more questions / comments I have:
>
> 1. Double equality checks all throughout the project - why are you using the == operator?

Such as?   There are a few enum's and singleton's so perhaps that explains it.

> 2. In TestShapesGet.getContext, the actual test to run is being selected randomly - why??

I ultimately want the shape geo tests to run with different DistanceCalculators that should be compatible (within a certain error epsilon anyway).  Picking one at random is my strategy to achieve this.  If I had more time, I might do JUnit's "Parameterized" test runner that could run a test class with a variety of differently configured contexts.  I'm welcome to your input of course.

~ David

> _______________________________________________
> dev mailing list
> [hidden email]
> http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com

_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com
Reply | Threaded
Open this post in threaded view
|

Re: [Dev] Some things we noted while porting

Itamar Syn-Hershko
I meant double, the type, not checking equality twice.

Basically you don't use the == operator, you use an epsilon or if (!(d1 < d2) && !(d2 < d1))

There are many places that do this -  GeodesicSphereDistCalc, DistanceUtils, and more

On Sun, Apr 29, 2012 at 8:12 AM, David Smiley <[hidden email]> wrote:

On Apr 28, <a href="tel:2012" value="+9722012">2012, at 7:51 PM, Itamar Syn-Hershko wrote:

> A few more questions / comments I have:
>
> 1. Double equality checks all throughout the project - why are you using the == operator?

Such as?   There are a few enum's and singleton's so perhaps that explains it.

> 2. In TestShapesGet.getContext, the actual test to run is being selected randomly - why??

I ultimately want the shape geo tests to run with different DistanceCalculators that should be compatible (within a certain error epsilon anyway).  Picking one at random is my strategy to achieve this.  If I had more time, I might do JUnit's "Parameterized" test runner that could run a test class with a variety of differently configured contexts.  I'm welcome to your input of course.

~ David

> _______________________________________________
> dev mailing list
> [hidden email]
> http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com

_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com



_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com
Reply | Threaded
Open this post in threaded view
|

Re: [Dev] Some things we noted while porting

dsmiley
So you mean the inverse of what you originally said, instead of "why are you using the == operator?" you meant "why aren't you using the == operator?"  -- ?  The only place in GeodesicSphereDistCalc I can see anything of this sort is in the standard equals() method -- which IntelliJ auto-generated for me.  It turns out Double.equals() guards against the NaN case and -0.0d vs 0.0d.

Aside from this, I can't find more cases in the 2 classes you referenced.

On Mon, Apr 30, 2012 at 5:11 AM, Itamar Syn-Hershko <[hidden email]> wrote:
I meant double, the type, not checking equality twice.

Basically you don't use the == operator, you use an epsilon or if (!(d1 < d2) && !(d2 < d1))

There are many places that do this -  GeodesicSphereDistCalc, DistanceUtils, and more

On Sun, Apr 29, 2012 at 8:12 AM, David Smiley <[hidden email]> wrote:

On Apr 28, <a href="tel:2012" value="+9722012" target="_blank">2012, at 7:51 PM, Itamar Syn-Hershko wrote:

> A few more questions / comments I have:
>
> 1. Double equality checks all throughout the project - why are you using the == operator?

Such as?   There are a few enum's and singleton's so perhaps that explains it.

> 2. In TestShapesGet.getContext, the actual test to run is being selected randomly - why??

I ultimately want the shape geo tests to run with different DistanceCalculators that should be compatible (within a certain error epsilon anyway).  Picking one at random is my strategy to achieve this.  If I had more time, I might do JUnit's "Parameterized" test runner that could run a test class with a variety of differently configured contexts.  I'm welcome to your input of course.

~ David

> _______________________________________________
> dev mailing list
> [hidden email]
> http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com

_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com



_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com



_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com
Reply | Threaded
Open this post in threaded view
|

Re: [Dev] Some things we noted while porting

Itamar Syn-Hershko
No, it's still  "why are you using the == operator? "

You _should_ be using the methods I mentioned instead of ==. See DistanceUtils for example, if (lat1 == lat2 && lon1 == lon2) is one example of many. I have Resharper shouting about "Comparison of floating point numbers with equality operator. Possible loss of precision while rounding values. Spatial is rounding values as it is, we probably want to avoid reducing accuracy further?


On Mon, Apr 30, 2012 at 5:58 PM, [hidden email] <[hidden email]> wrote:
So you mean the inverse of what you originally said, instead of "why are you using the == operator?" you meant "why aren't you using the == operator?"  -- ?  The only place in GeodesicSphereDistCalc I can see anything of this sort is in the standard equals() method -- which IntelliJ auto-generated for me.  It turns out Double.equals() guards against the NaN case and -0.0d vs 0.0d.

Aside from this, I can't find more cases in the 2 classes you referenced.


On Mon, Apr 30, <a href="tel:2012" value="+9722012" target="_blank">2012 at 5:11 AM, Itamar Syn-Hershko <[hidden email]> wrote:
I meant double, the type, not checking equality twice.

Basically you don't use the == operator, you use an epsilon or if (!(d1 < d2) && !(d2 < d1))

There are many places that do this -  GeodesicSphereDistCalc, DistanceUtils, and more

On Sun, Apr 29, <a href="tel:2012" value="+9722012" target="_blank">2012 at 8:12 AM, David Smiley <[hidden email]> wrote:

On Apr 28, <a href="tel:2012" value="+9722012" target="_blank">2012, at 7:51 PM, Itamar Syn-Hershko wrote:

> A few more questions / comments I have:
>
> 1. Double equality checks all throughout the project - why are you using the == operator?

Such as?   There are a few enum's and singleton's so perhaps that explains it.

> 2. In TestShapesGet.getContext, the actual test to run is being selected randomly - why??

I ultimately want the shape geo tests to run with different DistanceCalculators that should be compatible (within a certain error epsilon anyway).  Picking one at random is my strategy to achieve this.  If I had more time, I might do JUnit's "Parameterized" test runner that could run a test class with a variety of differently configured contexts.  I'm welcome to your input of course.

~ David

> _______________________________________________
> dev mailing list
> [hidden email]
> http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com

_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com



_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com



_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com



_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com
Reply | Threaded
Open this post in threaded view
|

Re: [Dev] Some things we noted while porting

dsmiley

On Mon, Apr 30, 2012 at 4:08 PM, Itamar Syn-Hershko <[hidden email]> wrote:
No, it's still  "why are you using the == operator? "

You _should_ be using the methods I mentioned instead of ==. See DistanceUtils for example, if (lat1 == lat2 && lon1 == lon2) is one example of many. I have Resharper shouting about "Comparison of floating point numbers with equality operator. Possible loss of precision while rounding values. Spatial is rounding values as it is, we probably want to avoid reducing accuracy further?

Oh that's just a quick short-circuit to keep certain identity cases from being munged if even slightly.  If you can find an example where the logic doesn't appear to be a quick short-circuit and you think there is a possible bug then please point out a specific reference.

~ David

_______________________________________________
dev mailing list
[hidden email]
http://lists.spatial4j.com/listinfo.cgi/dev-spatial4j.com