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

Robert Muir commented on LUCENE-8364:
-------------------------------------

Just looking, I have a few concerns:
* what is the goal of all the new abstractions? Abstractions have a significant 
cost, and I don't think we should be building a geo library here. We should 
just make the searches and stuff work.
* why does Polygon have new methods such as relate(), relatePoint() that are 
not used anywhere. We shouldn't add unnecessary stuff like that, we should keep 
this stuff minimal.
* the hashcode/equals on Polygon2d is unnecessary. It is an implementation 
detail and such methods should not be used. For example all queries just use 
equals() with the Polygon.
* methods like maxLon() on Polygon are unnecessary. These are already final 
variables so we don't need to wrap them in methods. Additionally such method 
names don't follow standard java notation: it seems to just add noise.
* some of the checks e.g. in Polygon are unnecessary. We don't need 
checkVertexIndex when the user already gets a correct exception 
(IndexOutOfBounds).

Maybe, it would be easier to split up the proposed changes so its easier to 
review. Especially for proposing any new abstract classes as I want to make 
sure that we really get value out of any abstractions, due to their high cost.

> Refactor and clean up core geo api
> ----------------------------------
>
>                 Key: LUCENE-8364
>                 URL: https://issues.apache.org/jira/browse/LUCENE-8364
>             Project: Lucene - Core
>          Issue Type: Improvement
>            Reporter: Nicholas Knize
>            Priority: Major
>         Attachments: LUCENE-8364.patch
>
>
> The core geo API is quite disorganized and confusing. For example there is 
> {{Polygon}} for creating an instance of polygon vertices and holes and 
> {{Polygon2D}} for computing relations between points and polygons. There is 
> also a {{PolygonPredicate}} and {{DistancePredicate}} in {{GeoUtils}} for 
> computing point in polygon and point distance relations, respectively, and a 
> {{GeoRelationUtils}} utility class which is no longer used for anything. This 
> disorganization is due to the organic improvements of simple {{LatLonPoint}} 
> indexing and search features and a little TLC is needed to clean up api to 
> make it more approachable and easy to understand. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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

Reply via email to