[prev in list] [next in list] [prev in thread] [next in thread] 

List:       lucene-dev
Subject:    [jira] Commented: (LUCENE-2148) Improve Spatial Point2D and
From:       "Simon Willnauer (JIRA)" <jira () apache ! org>
Date:       2009-12-28 18:21:29
Message-ID: 155527084.1262024489562.JavaMail.jira () brutus ! apache ! org
[Download RAW message or body]


    [ https://issues.apache.org/jira/browse/LUCENE-2148?page=com.atlassian.jira.plugin \
.system.issuetabpanels:comment-tabpanel&focusedCommentId=12794846#action_12794846 ] 

Simon Willnauer commented on LUCENE-2148:
-----------------------------------------

Chris, a couple of comments on the latest patch:

We should remove the flux warning from new classes IMO.
In LatLngRectangle we should make 180 and 90 doubles for consistency.

those two local vars are unnecessary and meaningless IMO could move in a bit down in \
the code, see snippet?! {code}
 private static LatLng boxCorner(LatLng centre, double width, double degrees) {
   double a = centre.getLat();
   double b = centre.getLng();
...
// could move them into this calc?!
//  double lat1 = Math.PI *  centre.getLat()  / 180d;
//  double lng1 = Math.PI * centre.getLng() / 180d;
{code}

We could simplify some conditional statments like the snipped below

{code}
private static LatLng normLat(double lat, double lng) {
...
 if (lng < 0) {
   lng = lng + 180;
  } else {
  lng = lng - 180;
}
// simplify those conditionals 
  lng += lng < 0 ? 180d: -180d; // something like that 
{code}

In LatLng should we make those degree constants double instead of int?!
{code}
public class LatLng {
 private final static int LONGITUDE_DECIMAL_DEGREE_RANGE = 360;
 private final static int LATITUDE_DECIMAL_DEGREE_RANGE = 180;
{code}

The deprecation comments should say something like:
@deprecated this class is unused and will be removed in a future release.

if we have a replacement for the class / method use something like:
@deprecated use Foo#bar(int) instead. This method / class will be removed in a future \
release

One more thing, I'm not sure if the @Deprecated annotations are necessary though \
@deprecated in javadoc seems to be the lucene way so far. Not sure about that though.


Except of those very minor things this seems to be very close to be ready! 







> Improve Spatial Point2D and Rectangle Classes
> ---------------------------------------------
> 
> Key: LUCENE-2148
> URL: https://issues.apache.org/jira/browse/LUCENE-2148
> Project: Lucene - Java
> Issue Type: Improvement
> Components: contrib/spatial
> Affects Versions: 3.1
> Reporter: Chris Male
> Assignee: Simon Willnauer
> Attachments: LUCENE-2148.patch, LUCENE-2148.patch, LUCENE-2148.patch
> 
> 
> The Point2D and Rectangle classes have alot of duplicate, redundant and used \
> functionality.  This issue cleans them both up and simplifies the functionality \
> they provide. Subsequent to this, Eclipse and LineSegment, which depend on Point2D, \
> are not used anywhere in the contrib, therefore rather than trying to update them \
> to use the improved Point2D, they will be removed.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


[prev in list] [next in list] [prev in thread] [next in thread]