[ 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