[jira] [Commented] (LUCENE-7152) Refactor lucene-spatial GeoUtils to core

["Robert Muir (JIRA)" <ji...@apache.org>]

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

Robert Muir commented on LUCENE-7152:
-------------------------------------

I dont think we need to bring complexity to the user just to have sane apis. Making sandbox/ and geo3d/ and who knows what else depend on spatial/ is confusing. 

The main advantage of stuff like {{checkLatitude}} and {{checkLongitude}} is that it helps detect and prevent a very common trap caused by the immaturity of the geospatial world. I know it makes sense to them somehow in their insane brains, that some things should take lat/lon and others lon/lat, but in practice this causes major issues. So adding these parameter checks is a huge win for usability, and I do not think we should have to incur a huge usability loss (crazy module dependencies) to have it.

I don't think we are talking about a lot of code here. For example the entirety of LatLonPoint/GeoPointField 2D geo just needs to be able to do basic stuff like form a bounding box (for distance query and efficient sorting), compute if something is in a polygon, etc. Its really just a few methods and actually the api surface area can be reduced more: GeoPointField can be refactored to look more like LatLonPoint: just one public class to do everything, and we can make all its queries etc package-private too.

All in all, what are we talking about here? like 5 public classes to have this in a {{.geo}} package in core? I think in this day and age it is reasonable to treat this stuff as a core capability, it covers the majority use case and extra modules can have more esoteric features? I also think its important that other modules can do basic things efficiently without all of them depending on spatial. For example expressions lets you incorporate distance but it does not do it as efficiently as LatLonPoint's distance sort, but maybe it could with some refactoring. Why should it have to depend on spatial/ to compute a simple bounding box? This kind of thing leads to inconsistencies and code duplication.


> Refactor lucene-spatial GeoUtils to core
> ----------------------------------------
>
>                 Key: LUCENE-7152
>                 URL: https://issues.apache.org/jira/browse/LUCENE-7152
>             Project: Lucene - Core
>          Issue Type: Bug
>            Reporter: Nicholas Knize
>         Attachments: LUCENE-7152.patch
>
>
> {{GeoUtils}} contains a lot of common spatial mathematics that can be reused across multiple packages. As discussed in LUCENE-7150 this issue will refactor GeoUtils to a new {{o.a.l.util.geo}} package in core that can be the home for other reusable spatial utility classes required by field and query implementations. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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